diff --git a/alchemy-json_api.gemspec b/alchemy-json_api.gemspec index f57b24cf..00430a50 100644 --- a/alchemy-json_api.gemspec +++ b/alchemy-json_api.gemspec @@ -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" diff --git a/app/controllers/alchemy/json_api/pages_controller.rb b/app/controllers/alchemy/json_api/pages_controller.rb index 9ee20b5e..31fc67a9 100644 --- a/app/controllers/alchemy/json_api/pages_controller.rb +++ b/app/controllers/alchemy/json_api/pages_controller.rb @@ -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 @@ -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 @@ -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) + 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 diff --git a/app/models/alchemy/json_api/page.rb b/app/models/alchemy/json_api/page.rb index f3d17780..8263ae78 100644 --- a/app/models/alchemy/json_api/page.rb +++ b/app/models/alchemy/json_api/page.rb @@ -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) diff --git a/spec/models/alchemy/json_api/page_spec.rb b/spec/models/alchemy/json_api/page_spec.rb index 0cabd5e9..21407d35 100644 --- a/spec/models/alchemy/json_api/page_spec.rb +++ b/spec/models/alchemy/json_api/page_spec.rb @@ -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 diff --git a/spec/requests/alchemy/json_api/pages_spec.rb b/spec/requests/alchemy/json_api/pages_spec.rb index 624024cd..62997867 100644 --- a/spec/requests/alchemy/json_api/pages_spec.rb +++ b/spec/requests/alchemy/json_api/pages_spec.rb @@ -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