From e6f6acafdaaa724d31b0be339739b9ee4299a286 Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Tue, 18 Jul 2023 12:42:44 +0200 Subject: [PATCH 1/7] Add support for autoloading nested related objects on ingredients This adds a `.preload_relations` object to serializers and leverages that and the Rails preloader service object to preload ActiveRecord object graphs below the related object of ingredients. --- .../alchemy/json_api/pages_controller.rb | 20 ++++++++++++++----- app/models/alchemy/json_api/page.rb | 17 ++++++++++++++++ .../alchemy/json_api/base_serializer.rb | 6 ++++++ .../json_api/ingredient_picture_serializer.rb | 4 ++++ .../alchemy/json_api/base_serializer_spec.rb | 6 ++++++ .../ingredient_picture_serializer_spec.rb | 6 ++++++ 6 files changed, 54 insertions(+), 5 deletions(-) diff --git a/app/controllers/alchemy/json_api/pages_controller.rb b/app/controllers/alchemy/json_api/pages_controller.rb index 9ee20b5e..a6f07016 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 @@ -109,6 +111,14 @@ def page_scope_with_includes ) end + def preload_ingredients(scope) + if params[:include]&.match?(/ingredients/) + Alchemy::JsonApi::Page.preload_ingredient_relations(scope, page_version_type) + else + scope + end + 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..aa034b42 100644 --- a/app/models/alchemy/json_api/page.rb +++ b/app/models/alchemy/json_api/page.rb @@ -1,6 +1,23 @@ module Alchemy module JsonApi class Page < SimpleDelegator + def self.preload_ingredient_relations(pages, page_version_type) + pages.map { |page| page.send(page_version_type) }.flat_map(&:elements).flat_map(&:ingredients).group_by do |ingredient| + "Alchemy::JsonApi::Ingredient#{ingredient.type.demodulize}Serializer".constantize.preload_relations + end.each do |preload_relations, ingredients| + preload(records: ingredients.map(&:related_object).compact, associations: preload_relations) + end + pages + end + + def self.preload(records:, associations:) + if Rails::VERSION::MAJOR >= 7 + ActiveRecord::Associations::Preloader.new(records: records, associations: associations).call + else + ActiveRecord::Associations::Preloader.new.preload(records, associations) + end + end + attr_reader :page_version_type, :page_version def initialize(page, page_version_type: :public_version) diff --git a/app/serializers/alchemy/json_api/base_serializer.rb b/app/serializers/alchemy/json_api/base_serializer.rb index 18cae81b..e227feae 100644 --- a/app/serializers/alchemy/json_api/base_serializer.rb +++ b/app/serializers/alchemy/json_api/base_serializer.rb @@ -4,6 +4,12 @@ class BaseSerializer include JSONAPI::Serializer set_key_transform Alchemy::JsonApi.key_transform + + # This method can be overwritten in individual serializers to fetch objects that belong to the related object in some form. + # This takes an Array of relation names that can be passed to the Rails preloader. + def self.preload_relations + [] + end end end end diff --git a/app/serializers/alchemy/json_api/ingredient_picture_serializer.rb b/app/serializers/alchemy/json_api/ingredient_picture_serializer.rb index 0a1291e3..d9b2b410 100644 --- a/app/serializers/alchemy/json_api/ingredient_picture_serializer.rb +++ b/app/serializers/alchemy/json_api/ingredient_picture_serializer.rb @@ -7,6 +7,10 @@ module JsonApi class IngredientPictureSerializer < BaseSerializer include IngredientSerializer + def self.preload_relations + [:thumbs] + end + attributes( :title, :caption, diff --git a/spec/serializers/alchemy/json_api/base_serializer_spec.rb b/spec/serializers/alchemy/json_api/base_serializer_spec.rb index e45890d6..42bbc3aa 100644 --- a/spec/serializers/alchemy/json_api/base_serializer_spec.rb +++ b/spec/serializers/alchemy/json_api/base_serializer_spec.rb @@ -9,4 +9,10 @@ expect(described_class).to receive(:set_key_transform).with(:underscore) load Alchemy::JsonApi::Engine.root.join("app/serializers/alchemy/json_api/base_serializer.rb") end + + describe ".preload_relations" do + subject { described_class.preload_relations } + + it { is_expected.to eq([]) } + end end diff --git a/spec/serializers/alchemy/json_api/ingredient_picture_serializer_spec.rb b/spec/serializers/alchemy/json_api/ingredient_picture_serializer_spec.rb index 90004c0a..2bfb53db 100644 --- a/spec/serializers/alchemy/json_api/ingredient_picture_serializer_spec.rb +++ b/spec/serializers/alchemy/json_api/ingredient_picture_serializer_spec.rb @@ -18,6 +18,12 @@ it_behaves_like "an ingredient serializer" + describe ".preload_relations" do + subject { described_class.preload_relations } + + it { is_expected.to eq([:thumbs]) } + end + describe "attributes" do subject { serializer.serializable_hash[:data][:attributes] } From eb62751d26d17ad56297ad0dc4977b0cbd225a6e Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Thu, 20 Jul 2023 17:16:53 +0200 Subject: [PATCH 2/7] Import fixes for pagination from jsonapi.rb PR See https://github.com/stas/jsonapi.rb/pull/91 for details on what these fix. --- .../alchemy/json_api/pages_controller.rb | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/app/controllers/alchemy/json_api/pages_controller.rb b/app/controllers/alchemy/json_api/pages_controller.rb index a6f07016..f5a5376a 100644 --- a/app/controllers/alchemy/json_api/pages_controller.rb +++ b/app/controllers/alchemy/json_api/pages_controller.rb @@ -154,6 +154,42 @@ def current_language def jsonapi_serializer_class(_resource, _is_collection) ::Alchemy::JsonApi::PageSerializer end + + # These overrides have to be in place until + # https://github.com/stas/jsonapi.rb/pull/91 + # is merged and released + def jsonapi_paginate(resources) + @_jsonapi_original_size = resources.size + super + end + + def jsonapi_pagination_meta(resources) + return {} unless JSONAPI::Rails.is_collection?(resources) + + _, limit, page = jsonapi_pagination_params + + numbers = { current: page } + + total = @_jsonapi_original_size + + last_page = [1, (total.to_f / limit).ceil].max + + if page > 1 + numbers[:first] = 1 + numbers[:prev] = page - 1 + end + + if page < last_page + numbers[:next] = page + 1 + numbers[:last] = last_page + end + + if total.present? + numbers[:records] = total + end + + numbers + end end end end From 17829a16a1c2522a415e20939c447b96a632c15c Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 1 Sep 2026 12:54:08 +0200 Subject: [PATCH 3/7] style: fix hash brace spacing flagged by standard The pagination fixes imported from the jsonapi.rb PR left spaces inside the hash literal braces, tripping Standard's Layout/SpaceInsideHashLiteralBraces cop and blocking CI. Remove them to match the project's lint config. --- app/controllers/alchemy/json_api/pages_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/alchemy/json_api/pages_controller.rb b/app/controllers/alchemy/json_api/pages_controller.rb index f5a5376a..61e4958c 100644 --- a/app/controllers/alchemy/json_api/pages_controller.rb +++ b/app/controllers/alchemy/json_api/pages_controller.rb @@ -168,7 +168,7 @@ def jsonapi_pagination_meta(resources) _, limit, page = jsonapi_pagination_params - numbers = { current: page } + numbers = {current: page} total = @_jsonapi_original_size From fcd5dec6adddeff80a7b395d8d00b7ae41e6e292 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 1 Sep 2026 12:54:14 +0200 Subject: [PATCH 4/7] refactor: harden ingredient relation preloading Guard the serializer lookup with safe_constantize so an ingredient type without a matching JSON:API serializer is skipped rather than raising a NameError during preloading. Also drop the Rails 6 Preloader fallback, which is dead code since the gem requires alchemy_cms >= 8.2, and that in turn requires Rails 7.2 or newer. --- app/models/alchemy/json_api/page.rb | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/app/models/alchemy/json_api/page.rb b/app/models/alchemy/json_api/page.rb index aa034b42..4a0668f8 100644 --- a/app/models/alchemy/json_api/page.rb +++ b/app/models/alchemy/json_api/page.rb @@ -3,7 +3,8 @@ module JsonApi class Page < SimpleDelegator def self.preload_ingredient_relations(pages, page_version_type) pages.map { |page| page.send(page_version_type) }.flat_map(&:elements).flat_map(&:ingredients).group_by do |ingredient| - "Alchemy::JsonApi::Ingredient#{ingredient.type.demodulize}Serializer".constantize.preload_relations + serializer = "Alchemy::JsonApi::Ingredient#{ingredient.type.demodulize}Serializer".safe_constantize + serializer ? serializer.preload_relations : [] end.each do |preload_relations, ingredients| preload(records: ingredients.map(&:related_object).compact, associations: preload_relations) end @@ -11,11 +12,7 @@ def self.preload_ingredient_relations(pages, page_version_type) end def self.preload(records:, associations:) - if Rails::VERSION::MAJOR >= 7 - ActiveRecord::Associations::Preloader.new(records: records, associations: associations).call - else - ActiveRecord::Associations::Preloader.new.preload(records, associations) - end + ActiveRecord::Associations::Preloader.new(records: records, associations: associations).call end attr_reader :page_version_type, :page_version From 75d288841917740c4ce8b92252da77c195bde443 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Wed, 2 Sep 2026 08:12:45 +0200 Subject: [PATCH 5/7] refactor: drop jsonapi.rb pagination overrides The jsonapi_paginate and jsonapi_pagination_meta overrides were a workaround for jsonapi.rb losing the pre-pagination total when the collection is decorated after pagination (the pages controller maps the paginated result into api_page objects). That was fixed upstream by stas/jsonapi.rb#91, first released in jsonapi.rb 2.1.1, whose implementation is now identical to the overrides. Require >= 2.1.1 so the fix is guaranteed to be present and remove the redundant code. --- alchemy-json_api.gemspec | 2 +- .../alchemy/json_api/pages_controller.rb | 36 ------------------- 2 files changed, 1 insertion(+), 37 deletions(-) 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 61e4958c..a6f07016 100644 --- a/app/controllers/alchemy/json_api/pages_controller.rb +++ b/app/controllers/alchemy/json_api/pages_controller.rb @@ -154,42 +154,6 @@ def current_language def jsonapi_serializer_class(_resource, _is_collection) ::Alchemy::JsonApi::PageSerializer end - - # These overrides have to be in place until - # https://github.com/stas/jsonapi.rb/pull/91 - # is merged and released - def jsonapi_paginate(resources) - @_jsonapi_original_size = resources.size - super - end - - def jsonapi_pagination_meta(resources) - return {} unless JSONAPI::Rails.is_collection?(resources) - - _, limit, page = jsonapi_pagination_params - - numbers = {current: page} - - total = @_jsonapi_original_size - - last_page = [1, (total.to_f / limit).ceil].max - - if page > 1 - numbers[:first] = 1 - numbers[:prev] = page - 1 - end - - if page < last_page - numbers[:next] = page + 1 - numbers[:last] = last_page - end - - if total.present? - numbers[:records] = total - end - - numbers - end end end end From 03c7d75febdaa016db4e4a131dc793e0b397f460 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Wed, 2 Sep 2026 12:38:44 +0200 Subject: [PATCH 6/7] fix: preload ingredient related objects via the storage adapter The picture serializer hard-coded `preload_relations => [:thumbs]`, but `Alchemy::Picture` only defines the `thumbs` association under the Dragonfly storage adapter. On the default ActiveStorage adapter that association does not exist, so serializing a picture ingredient with a picture attached raised `ActiveRecord::AssociationNotFoundError` and returned a 500 for any request including ingredients. No spec attached a real picture, so the suite missed it. Instead of naming associations per serializer, ask each related object class to preload its own storage-specific associations through Alchemy's `alchemy_element_preloads` hook (the same mechanism core's ElementPreloader uses), which resolves to the correct associations for the active storage adapter and works for any relatable resource. This removes the `preload_relations` serializer API entirely. --- app/models/alchemy/json_api/page.rb | 16 +++++++--------- .../alchemy/json_api/base_serializer.rb | 6 ------ .../json_api/ingredient_picture_serializer.rb | 4 ---- spec/models/alchemy/json_api/page_spec.rb | 18 ++++++++++++++++++ .../alchemy/json_api/base_serializer_spec.rb | 6 ------ .../ingredient_picture_serializer_spec.rb | 6 ------ 6 files changed, 25 insertions(+), 31 deletions(-) diff --git a/app/models/alchemy/json_api/page.rb b/app/models/alchemy/json_api/page.rb index 4a0668f8..8263ae78 100644 --- a/app/models/alchemy/json_api/page.rb +++ b/app/models/alchemy/json_api/page.rb @@ -2,19 +2,17 @@ module Alchemy module JsonApi class Page < SimpleDelegator def self.preload_ingredient_relations(pages, page_version_type) - pages.map { |page| page.send(page_version_type) }.flat_map(&:elements).flat_map(&:ingredients).group_by do |ingredient| - serializer = "Alchemy::JsonApi::Ingredient#{ingredient.type.demodulize}Serializer".safe_constantize - serializer ? serializer.preload_relations : [] - end.each do |preload_relations, ingredients| - preload(records: ingredients.map(&:related_object).compact, associations: preload_relations) + 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 - def self.preload(records:, associations:) - ActiveRecord::Associations::Preloader.new(records: records, associations: associations).call - end - attr_reader :page_version_type, :page_version def initialize(page, page_version_type: :public_version) diff --git a/app/serializers/alchemy/json_api/base_serializer.rb b/app/serializers/alchemy/json_api/base_serializer.rb index e227feae..18cae81b 100644 --- a/app/serializers/alchemy/json_api/base_serializer.rb +++ b/app/serializers/alchemy/json_api/base_serializer.rb @@ -4,12 +4,6 @@ class BaseSerializer include JSONAPI::Serializer set_key_transform Alchemy::JsonApi.key_transform - - # This method can be overwritten in individual serializers to fetch objects that belong to the related object in some form. - # This takes an Array of relation names that can be passed to the Rails preloader. - def self.preload_relations - [] - end end end end diff --git a/app/serializers/alchemy/json_api/ingredient_picture_serializer.rb b/app/serializers/alchemy/json_api/ingredient_picture_serializer.rb index d9b2b410..0a1291e3 100644 --- a/app/serializers/alchemy/json_api/ingredient_picture_serializer.rb +++ b/app/serializers/alchemy/json_api/ingredient_picture_serializer.rb @@ -7,10 +7,6 @@ module JsonApi class IngredientPictureSerializer < BaseSerializer include IngredientSerializer - def self.preload_relations - [:thumbs] - end - attributes( :title, :caption, 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/serializers/alchemy/json_api/base_serializer_spec.rb b/spec/serializers/alchemy/json_api/base_serializer_spec.rb index 42bbc3aa..e45890d6 100644 --- a/spec/serializers/alchemy/json_api/base_serializer_spec.rb +++ b/spec/serializers/alchemy/json_api/base_serializer_spec.rb @@ -9,10 +9,4 @@ expect(described_class).to receive(:set_key_transform).with(:underscore) load Alchemy::JsonApi::Engine.root.join("app/serializers/alchemy/json_api/base_serializer.rb") end - - describe ".preload_relations" do - subject { described_class.preload_relations } - - it { is_expected.to eq([]) } - end end diff --git a/spec/serializers/alchemy/json_api/ingredient_picture_serializer_spec.rb b/spec/serializers/alchemy/json_api/ingredient_picture_serializer_spec.rb index 2bfb53db..90004c0a 100644 --- a/spec/serializers/alchemy/json_api/ingredient_picture_serializer_spec.rb +++ b/spec/serializers/alchemy/json_api/ingredient_picture_serializer_spec.rb @@ -18,12 +18,6 @@ it_behaves_like "an ingredient serializer" - describe ".preload_relations" do - subject { described_class.preload_relations } - - it { is_expected.to eq([:thumbs]) } - end - describe "attributes" do subject { serializer.serializable_hash[:data][:attributes] } From 76724a0d5be9d1dd836ee76096e5224f8d5720f7 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Wed, 2 Sep 2026 13:08:58 +0200 Subject: [PATCH 7/7] perf: only eager load ingredients when they are included Ingredient relationships are lazy-loaded, so their linkage and resources are only emitted when the request's `include` asks for them. The scope, however, eager loaded `{ingredients: :related_object}` on every request, materialising ingredients and their related objects even for requests that never serialize them. Gate that eager load (and the matching related-object preload) on the same `include_ingredients?` predicate so both stay in sync and nothing is fetched needlessly. The element tree stays eager because its linkage is always present. --- .../alchemy/json_api/pages_controller.rb | 29 ++++++++++--------- spec/requests/alchemy/json_api/pages_spec.rb | 26 +++++++++++++++++ 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/app/controllers/alchemy/json_api/pages_controller.rb b/app/controllers/alchemy/json_api/pages_controller.rb index a6f07016..31fc67a9 100644 --- a/app/controllers/alchemy/json_api/pages_controller.rb +++ b/app/controllers/alchemy/json_api/pages_controller.rb @@ -99,24 +99,27 @@ 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) - if params[:include]&.match?(/ingredients/) - Alchemy::JsonApi::Page.preload_ingredient_relations(scope, page_version_type) - else - scope - end + 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 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