Skip to content
Merged
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
1 change: 1 addition & 0 deletions alchemy_cms.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ Gem::Specification.new do |gem|
gem.add_development_dependency "selenium-webdriver", ["~> 4.10"]
gem.add_development_dependency "webmock", ["~> 3.3"]
gem.add_development_dependency "shoulda-matchers", "~> 7.0"
gem.add_development_dependency "db-query-matchers", "~> 0.12"
gem.add_development_dependency "timecop", ["~> 0.9"]

gem.post_install_message = <<~MSG
Expand Down
60 changes: 32 additions & 28 deletions app/controllers/alchemy/admin/elements_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,12 @@ class ElementsController < Alchemy::Admin::BaseController
authorize_resource class: Alchemy::Element

def index
elements = @page_version.elements.order(:position).includes(*element_includes)
@elements = elements.not_nested.unfixed
@fixed_elements = elements.not_nested.fixed
preloaded = Alchemy::ElementPreloader
.new(page_version: @page_version)
.call

@elements = preloaded.reject(&:fixed?)
@fixed_elements = preloaded.select(&:fixed?)
end

def new
Expand Down Expand Up @@ -130,12 +133,12 @@ def order
def collapse
# We do not want to trigger the touch callback or any validations
@element.update_columns(folded: true)
# Collapse all nested elements
nested_elements_ids = collapse_nested_elements_ids(@element)
Alchemy::Element.where(id: nested_elements_ids).update_all(folded: true)
# Collapse all nested elements (except compact ones which stay as-is)
nested_element_ids = nested_element_ids_to_collapse(@element)
Alchemy::Element.where(id: nested_element_ids).update_all(folded: true)

render json: {
nestedElementIds: nested_elements_ids,
nestedElementIds: nested_element_ids,
title: Alchemy.t(@element.folded? ? :show_element_content : :hide_element_content)
}
end
Expand Down Expand Up @@ -163,30 +166,31 @@ def load_page_and_version
@page = @page_version.page
end

def collapse_nested_elements_ids(element)
# Collects IDs of nested elements that should be collapsed.
# Skips compact elements (they retain their fold state).
# Optimized to use a single query instead of N queries for N levels.
def nested_element_ids_to_collapse(element)
all_elements = Element
.where(page_version_id: element.page_version_id)
.select(:id, :parent_element_id, :folded, :name)
.to_a

children_by_parent = all_elements.group_by(&:parent_element_id)

ids = []
element.all_nested_elements.includes(:all_nested_elements).reject(&:compact?).each do |nested_element|
ids.push nested_element.id if nested_element.expanded?
ids.concat collapse_nested_elements_ids(nested_element) if nested_element.all_nested_elements.reject(&:compact?).any?
stack = [element.id]

while stack.any?
current_id = stack.pop
children = children_by_parent[current_id] || []

children.reject(&:compact?).each do |child|
ids << child.id if child.expanded?
stack << child.id
end
end
ids
end

def element_includes
[
{
ingredients: :related_object
},
:tags,
{
all_nested_elements: [
{
ingredients: :related_object
},
:tags
]
}
]
ids
end

def load_element
Expand Down
14 changes: 11 additions & 3 deletions app/models/alchemy/element/element_ingredients.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,19 @@ def ingredient_definition_for(role)
# This is used to re-initialize the TinyMCE editor in the element editor.
#
def richtext_ingredients_ids
ids = ingredients.select(&:has_tinymce?).collect(&:id)
expanded_nested_elements = nested_elements.expanded
ids = ingredients.filter_map { |i| i.id if i.has_tinymce? }

# Use preloaded association if available, otherwise query
expanded_nested_elements = if association(:all_nested_elements).loaded?
all_nested_elements.select(&:expanded?)
else
nested_elements.expanded
end

if expanded_nested_elements.present?
ids += expanded_nested_elements.collect(&:richtext_ingredients_ids)
ids += expanded_nested_elements.map(&:richtext_ingredients_ids)
end

ids.flatten
end

Expand Down
19 changes: 18 additions & 1 deletion app/models/alchemy/picture.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,16 @@ def ransackable_scopes(_auth_object = nil)
def file_formats(scope = all)
Alchemy.storage_adapter.file_formats(name, scope:)
end

# Preload associations for element editor display
#
# @param pictures [Array<Picture>] Collection of pictures to preload for
def alchemy_element_preloads(pictures)
return if pictures.blank?

# Preload storage-specific associations to avoid N+1 when rendering thumbnails
Alchemy.storage_adapter.preload_picture_associations(pictures)
end
end

# Instance methods
Expand Down Expand Up @@ -183,8 +193,15 @@ def update_name_and_tag_list!(params)
end

# Returns the picture description for a given language.
#
# @param language [Language] The language to get description for
# @return [String, nil] The description text or nil
def description_for(language)
descriptions.find_by(language: language)&.text
if descriptions.loaded?
descriptions.detect { _1.language == language }&.text
else
descriptions.find_by(language: language)&.text
end
end

# Returns an uri escaped name.
Expand Down
1 change: 1 addition & 0 deletions app/models/alchemy/storage_adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class UnknownAdapterError < StandardError; end
:image_file_size,
:image_file_width,
:picture_url_class,
:preload_picture_associations,
:preloaded_pictures,
:preprocessor_class,
:ransackable_associations,
Expand Down
9 changes: 9 additions & 0 deletions app/models/alchemy/storage_adapter/active_storage.rb
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,15 @@ def preloaded_pictures(pictures)
pictures.with_attached_image_file
end

# Preload picture associations on already-loaded records
# @param [Array<Alchemy::Picture>] pictures
def preload_picture_associations(pictures)
ActiveRecord::Associations::Preloader.new(
records: pictures,
associations: {image_file_attachment: :blob}
).call
end

# @param [Alchemy::Attachment]
# @return [TrueClass, FalseClass]
def set_attachment_name?(attachment)
Expand Down
9 changes: 9 additions & 0 deletions app/models/alchemy/storage_adapter/dragonfly.rb
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,15 @@ def preloaded_pictures(pictures)
pictures.includes(:thumbs)
end

# Preload picture associations on already-loaded records
# @param [Array<Alchemy::Picture>] pictures
def preload_picture_associations(pictures)
ActiveRecord::Associations::Preloader.new(
records: pictures,
associations: :thumbs
).call
end

# @param [Alchemy::Attachment]
# @return [TrueClass, FalseClass]
def set_attachment_name?(attachment)
Expand Down
14 changes: 14 additions & 0 deletions app/models/concerns/alchemy/relatable_resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,20 @@ module Alchemy
module RelatableResource
extend ActiveSupport::Concern

class_methods do
# Preload associations for element editor display
#
# Override this method in models that need custom preloading
# when displayed in the element editor (e.g., preloading
# picture thumbnails or products).
#
# @param records [Array] Collection of records to preload for
def alchemy_element_preloads(records)
# Default implementation does nothing
# Override in subclasses that need preloading
end
end

included do
scope :deletable, -> do
where(
Expand Down
107 changes: 107 additions & 0 deletions app/services/alchemy/element_preloader.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
# frozen_string_literal: true

module Alchemy
# Preloads element trees with all associations and nested elements
#
# This service efficiently loads element trees to avoid N+1 queries.
# It recursively preloads all nested elements to unlimited depth.
#
# @example Preload elements for a page version
# preloader = Alchemy::ElementPreloader.new(page_version: page_version)
# preloaded_elements = preloader.call
#
class ElementPreloader
# @param page_version [PageVersion] The page version to preload elements for
def initialize(page_version:)
@page_version = page_version
ActiveRecord::Associations::Preloader.new(
records: [page_version],
associations: {page: :language}
).call
end

# Preloads and returns the element tree with all associations loaded
#
# @return [Array<Element>] Elements with preloaded nested elements
def call
# Load all elements for the page version with associations
all_elements = load_all_elements
return [] if all_elements.empty?

# Build parent -> children lookup and populate associations
populate_nested_associations(all_elements)

# Root elements are those without a parent
root_elements = all_elements.values
.select { |e| e.parent_element_id.nil? }
.sort_by(&:position)
return [] if root_elements.empty?

preload_related_objects(root_elements)

root_elements
end

private

attr_reader :page_version

# Load all elements for the page version and preload their associations
def load_all_elements
Element
.where(page_version_id: page_version.id)
.includes(*element_includes)
.index_by(&:id)
end

# Populate the all_nested_elements association for each element
def populate_nested_associations(elements_by_id)
# Group elements by parent_id
elements_by_parent = elements_by_id.values.group_by(&:parent_element_id)

elements_by_id.each_value do |element|
children = elements_by_parent[element.id] || []
children = children.sort_by(&:position)

# Manually set the association target
element.association(:all_nested_elements).target = children
element.association(:all_nested_elements).loaded!
end
end

# Associations to preload for element rendering
def element_includes
[
{ingredients: :related_object},
:tags
]
end

# Preload related objects for all ingredients in elements
# Allows related objects to preload their associations (e.g., picture thumbnails)
def preload_related_objects(root_elements)
related_objects_by_class = collect_related_objects(root_elements)
return if related_objects_by_class.empty?

related_objects_by_class.each do |klass, objects|
if klass.respond_to?(:alchemy_element_preloads)
klass.alchemy_element_preloads(objects)
end
end
end

# Collect unique related objects from element tree, grouped by class
def collect_related_objects(elements, collected = Hash.new { |h, k| h[k] = {} })
elements.each do |element|
element.ingredients.each do |ingredient|
obj = ingredient.related_object
collected[obj.class][obj.id] = obj if obj
end
if element.association(:all_nested_elements).loaded?
collect_related_objects(element.all_nested_elements, collected)
end
end
collected.transform_values(&:values)
end
end
end
13 changes: 13 additions & 0 deletions spec/controllers/alchemy/admin/elements_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,19 @@ module Alchemy
"title" => "Show content of this element."
})
end

it "uses bounded queries regardless of nesting depth" do
query_count = 0
counter = ->(*, _) { query_count += 1 }

ActiveSupport::Notifications.subscribed(counter, "sql.active_record") do
subject
end

# Should use a small bounded number of queries (includes auth, session, etc.):
# Key point: query count doesn't grow with nesting depth
expect(query_count).to be <= 10
end
end
end

Expand Down
16 changes: 16 additions & 0 deletions spec/models/alchemy/element_ingredients_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,22 @@
nested_element_2.ingredient_ids
)
end

context "when all_nested_elements is preloaded" do
subject do
Alchemy::Element.preload(:all_nested_elements, ingredients: :related_object)
.find(element.id)
.richtext_ingredients_ids
end

it "includes all richtext ingredients from all expanded descendent elements" do
is_expected.to eq(
element.ingredient_ids +
nested_element_1.ingredient_ids +
nested_element_2.ingredient_ids
)
end
end
end
end
end
Loading
Loading