Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion alchemy-json_api.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Gem::Specification.new do |spec|
spec.files = Dir["{app,config,db,lib}/**/*", "LICENSE", "Rakefile", "README.md"]

spec.add_dependency "alchemy_cms", [">= 8.3.0", "< 9"]
spec.add_dependency "jsonapi.rb", [">= 1.6.0", "< 2.2"]
spec.add_dependency "jsonapi.rb", [">= 2.1.1", "< 2.2"]

spec.add_development_dependency "factory_bot"
spec.add_development_dependency "jsonapi-rspec"
Expand Down
39 changes: 26 additions & 13 deletions app/controllers/alchemy/json_api/pages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ def show
def render_pages_json(allowed)
# Only load pages with all includes when browser cache is stale
jsonapi_filter(page_scope_with_includes, allowed) do |filtered|
# decorate with our page model that has a eager loaded elements collection
filtered_pages = filtered.result.map { |page| api_page(page) }
jsonapi_paginate(filtered_pages) do |paginated|
render jsonapi: paginated
jsonapi_paginate(filtered.result) do |paginated|
# decorate with our page model that has a eager loaded elements collection
decorated_pages = preload_ingredients(paginated).map { |page| api_page(page) }
render jsonapi: decorated_pages
end
end
end
Expand Down Expand Up @@ -74,7 +74,9 @@ def jsonapi_meta(pages)
end

def load_page
@page = load_page_by_id || load_page_by_urlname || raise(ActiveRecord::RecordNotFound)
@page = preload_ingredients(
[load_page_by_id || load_page_by_urlname || raise(ActiveRecord::RecordNotFound)]
).first
end

def load_page_by_id
Expand All @@ -97,18 +99,29 @@ def page_scope_with_includes
[
:legacy_urls,
{language: {nodes: [:parent, :children, {page: {language: {site: :languages}}}]}},
{
page_version_type => {
elements: [
:nested_elements,
{ingredients: :related_object}
]
}
}
{page_version_type => {elements: element_includes}}
]
)
end

# Ingredients are lazy-loaded relationships, so only eager load them (and
# their related objects) when the request actually includes them.
def element_includes
includes = [:nested_elements]
includes << {ingredients: :related_object} if include_ingredients?
includes
end

def preload_ingredients(scope)
Comment thread
tvdeyen marked this conversation as resolved.
return scope unless include_ingredients?

Alchemy::JsonApi::Page.preload_ingredient_relations(scope, page_version_type)
end

def include_ingredients?
params[:include]&.match?(/ingredients/)
end

def page_version_type
:public_version
end
Expand Down
12 changes: 12 additions & 0 deletions app/models/alchemy/json_api/page.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,18 @@
module Alchemy
module JsonApi
class Page < SimpleDelegator
def self.preload_ingredient_relations(pages, page_version_type)
related_objects = pages.map { |page| page.send(page_version_type) }
.flat_map(&:elements)
.flat_map(&:ingredients)
.filter_map(&:related_object)

related_objects.group_by(&:class).each do |klass, objects|
klass.alchemy_element_preloads(objects) if klass.respond_to?(:alchemy_element_preloads)
end
pages
end

attr_reader :page_version_type, :page_version

def initialize(page, page_version_type: :public_version)
Expand Down
18 changes: 18 additions & 0 deletions spec/models/alchemy/json_api/page_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,22 @@
is_expected.to match([fixed_element.id])
end
end

describe ".preload_ingredient_relations" do
let(:page) { FactoryBot.create(:alchemy_page, :public) }
let(:element) { FactoryBot.create(:alchemy_element, page_version: page.public_version, name: "all_you_can_eat") }
let(:picture) { FactoryBot.create(:alchemy_picture) }
let!(:ingredient) { FactoryBot.create(:alchemy_ingredient_picture, element: element, picture: picture) }

it "lets each related object class preload its own storage-specific associations" do
expect(Alchemy::Picture).to receive(:alchemy_element_preloads).with([picture])
described_class.preload_ingredient_relations([page], :public_version)
end

it "preloads related objects without raising and returns the pages" do
expect {
expect(described_class.preload_ingredient_relations([page], :public_version)).to eq([page])
}.not_to raise_error
end
end
end
26 changes: 26 additions & 0 deletions spec/requests/alchemy/json_api/pages_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,32 @@ def count_element_load_queries

expect(queries_for_three_pages).to eq(queries_for_one_page)
end

def count_ingredient_load_queries
queries = []
subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |_name, _start, _finish, _id, payload|
next if payload[:name] == "SCHEMA" || payload[:cached]
queries << payload[:sql] if payload[:sql].include?(%("alchemy_ingredients"))
end
yield
queries.size
ensure
ActiveSupport::Notifications.unsubscribe(subscriber)
end

it "does not load ingredients when they are not included" do
create_page_with_element

without_ingredients = count_ingredient_load_queries do
get alchemy_json_api.pages_path(include: "all_elements")
end
expect(without_ingredients).to eq(0)

with_ingredients = count_ingredient_load_queries do
get alchemy_json_api.pages_path(include: "all_elements.ingredients")
end
expect(with_ingredients).to be > 0
end
end

context "with pagination params" do
Expand Down
Loading