Skip to content

Commit

Permalink
Remove eager loading of ingredients_associations
Browse files Browse the repository at this point in the history
To make this feature work we added a Hack into active record. This hack does not work in Rails 7 anymore.

Since contents with essences are deprecated anyway and we do not have this problem with ingredients it is not worth adjusting the hack to active record 7

This reverts commit c45a6dd
  • Loading branch information
tvdeyen committed Apr 11, 2022
1 parent 760b1d4 commit 2c9a785
Show file tree
Hide file tree
Showing 7 changed files with 8 additions and 62 deletions.
8 changes: 2 additions & 6 deletions app/controllers/alchemy/admin/elements_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,18 +106,14 @@ def fold
def element_includes
[
{
contents: {
essence: :ingredient_association,
},
contents: :essence,
ingredients: :related_object,
},
:tags,
{
all_nested_elements: [
{
contents: {
essence: :ingredient_association,
},
contents: :essence,
ingredients: :related_object,
},
:tags,
Expand Down
6 changes: 1 addition & 5 deletions app/controllers/alchemy/api/contents_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,7 @@ def show
private

def content_includes
[
{
essence: :ingredient_association,
},
]
%i[essence]
end
end
end
8 changes: 2 additions & 6 deletions app/controllers/alchemy/api/elements_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,14 @@ def element_includes
{
nested_elements: [
{
contents: {
essence: :ingredient_association,
},
contents: :essence,
ingredients: :related_object,
},
:tags,
],
},
{
contents: {
essence: :ingredient_association,
},
contents: :essence,
ingredients: :related_object,
},
:tags,
Expand Down
8 changes: 2 additions & 6 deletions app/controllers/alchemy/api/pages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,17 +112,13 @@ def page_includes
{
nested_elements: [
{
contents: {
essence: :ingredient_association,
},
contents: :essence,
},
:tags,
],
},
{
contents: {
essence: :ingredient_association,
},
contents: :essence,
},
:tags,
],
Expand Down
26 changes: 0 additions & 26 deletions lib/alchemy/essence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,6 @@
require "active_record"

module Alchemy #:nodoc:
# A bogus association that skips eager loading for essences not having an ingredient association
class IngredientAssociation < ActiveRecord::Associations::BelongsToAssociation
# Skip eager loading if called by Rails' preloader
def klass
if caller.any? { |line| line =~ /preloader\.rb/ }
nil
else
super
end
end
end

module Essence #:nodoc:
def self.included(base)
base.extend(ClassMethods)
Expand Down Expand Up @@ -43,8 +31,6 @@ def acts_as_essence(options = {})
ingredient_column: "body",
}.update(options)

@_classes_with_ingredient_association ||= []

class_eval <<-RUBY, __FILE__, __LINE__ + 1
attr_writer :validation_errors
include Alchemy::Essence::InstanceMethods
Expand Down Expand Up @@ -87,18 +73,6 @@ def preview_text_column
alias_method :#{configuration[:ingredient_column]}, :ingredient_association
alias_method :#{configuration[:ingredient_column]}=, :ingredient_association=
RUBY

@_classes_with_ingredient_association << self
end
end

# Overwrite ActiveRecords method to return a bogus association class that skips eager loading
# for essence classes that do not have an ingredient association
def _reflect_on_association(name)
if name == :ingredient_association && !in?(@_classes_with_ingredient_association)
OpenStruct.new(association_class: Alchemy::IngredientAssociation)
else
super
end
end

Expand Down
12 changes: 0 additions & 12 deletions lib/alchemy/test_support/essence_shared_examples.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,6 @@
let(:content) { Alchemy::Content.new(name: "foo") }
let(:content_definition) { { "name" => "foo" } }

describe "eager loading" do
before do
2.times { described_class.create! }
end

it "does not throw error if eager loaded" do
expect {
described_class.all.includes(:ingredient_association).to_a
}.to_not raise_error
end
end

it "touches the element after save" do
element = FactoryBot.create(:alchemy_element)
content = FactoryBot.create(:alchemy_content, element: element, essence: essence, essence_type: essence.class.name)
Expand Down
2 changes: 1 addition & 1 deletion lib/alchemy/upgrader/tasks/ingredients_migrator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def create_ingredients
# eager load all elements that have ingredients defined but no ingredient records yet.
all_elements = Alchemy::Element
.named(elements_with_ingredients.map { |d| d[:name] })
.includes(contents: { essence: :ingredient_association })
.includes(contents: :essence)
.left_outer_joins(:ingredients).where(alchemy_ingredients: { id: nil })
.to_a
elements_with_ingredients.map do |element_definition|
Expand Down

0 comments on commit 2c9a785

Please sign in to comment.