Skip to content

Commit

Permalink
Fixes #12011 - Replace .includes(table).where(table) by .eager_load
Browse files Browse the repository at this point in the history
On Rails 3, .includes(:users).where("users.name = 'daniel'") works because
Rails would be smart enough to figure out the where query uses the includes
table. In this case it'd internally resolve it to eager_load to use the table
users in the query.

However on Rails 4 they decided they didn't want to maintain a parser for SQL,
so we have to append .references(:table) after every
 .includes(:table).where("SQL that uses table").
This is not retrocompatible, but we can directly use eager_load on Rails 3 and
4 to avoid using .references.
  • Loading branch information
dLobatog committed Sep 30, 2015
1 parent 551bd73 commit 5aba0d1
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 11 deletions.
2 changes: 1 addition & 1 deletion app/models/compute_profile.rb
Expand Up @@ -20,5 +20,5 @@ class ComputeProfile < ActiveRecord::Base
scoped_search :on => :name, :complete_value => true
default_scope -> { order('compute_profiles.name') }

scope :visibles, -> { includes(:compute_attributes).where('compute_attributes.id > 0') }
scope :visibles, -> { eager_load(:compute_attributes).where('compute_attributes.id > 0') }
end
4 changes: 2 additions & 2 deletions app/models/fact_value.rb
Expand Up @@ -17,10 +17,10 @@ class FactValue < ActiveRecord::Base
scoped_search :in => :fact_name, :on => :short_name, :complete_value => true, :alias => "fact_short_name"

scope :no_timestamp_facts, lambda {
includes(:fact_name).where("fact_names.name <> ?",:_timestamp)
eager_load(:fact_name).where("fact_names.name <> ?",:_timestamp)
}
scope :timestamp_facts, lambda {
joins(:fact_name).where("fact_names.name = ?",:_timestamp)
eager_load(:fact_name).where("fact_names.name = ?",:_timestamp)
}
scope :my_facts, lambda {
if !User.current.admin? || Organization.expand(Organization.current).present? || Location.expand(Location.current).present?
Expand Down
2 changes: 1 addition & 1 deletion app/models/host/managed.rb
Expand Up @@ -82,7 +82,7 @@ class Jail < ::Safemode::Jail
scope :with_os, -> { where('hosts.operatingsystem_id IS NOT NULL') }

scope :with_status, lambda { |status_type|
includes(:host_statuses).where("host_status.type = '#{status_type}'")
eager_load(:host_statuses).where("host_status.type = '#{status_type}'")
}

scope :with_config_status, lambda {
Expand Down
8 changes: 4 additions & 4 deletions app/models/user.rb
Expand Up @@ -45,14 +45,14 @@ class User < ActiveRecord::Base
attr_name :login

scope :except_admin, lambda {
includes(:cached_usergroups).
eager_load(:cached_usergroups).
where(["(#{self.table_name}.admin = ? OR #{self.table_name}.admin IS NULL) AND " +
"(#{Usergroup.table_name}.admin = ? OR #{Usergroup.table_name}.admin IS NULL)",
false, false])
false, false])
}
scope :only_admin, lambda {
includes(:cached_usergroups).
where(["#{self.table_name}.admin = ? OR #{Usergroup.table_name}.admin = ?", true, true])
eager_load(:cached_usergroups).
where(["#{self.table_name}.admin = ? OR #{Usergroup.table_name}.admin = ?", true, true])
}
scope :except_hidden, lambda {
if (hidden = AuthSourceHidden.pluck('auth_sources.id')).present?
Expand Down
2 changes: 1 addition & 1 deletion app/services/authorizer.rb
Expand Up @@ -63,7 +63,7 @@ def find_collection(resource_class, options = {})
end
else
# build regular filtered scope for "resource_class"
scope = resource_class.includes(scope_components[:includes]).joins(scope_components[:joins]).readonly(false)
scope = resource_class.eager_load(scope_components[:includes]).joins(scope_components[:joins]).readonly(false)
scope_components[:where].inject(scope) { |scope_build,where| scope_build.where(where) }
end
end
Expand Down
4 changes: 2 additions & 2 deletions app/services/fact_importer.rb
Expand Up @@ -49,7 +49,7 @@ def fact_name_class
attr_reader :host, :facts

def delete_removed_facts
to_delete = host.fact_values.joins(:fact_name).where("fact_names.type = '#{fact_name_class}' AND fact_names.name NOT IN (?)", facts.keys)
to_delete = host.fact_values.eager_load(:fact_name).where("fact_names.type = '#{fact_name_class}' AND fact_names.name NOT IN (?)", facts.keys)
# N+1 DELETE SQL, but this would allow us to use callbacks (e.g. auditing) when deleting.
deleted = to_delete.destroy_all
@counters[:deleted] = deleted.size
Expand Down Expand Up @@ -96,6 +96,6 @@ def normalize(facts)
end

def db_facts
@db_facts ||= host.fact_values.includes(:fact_name).where("fact_names.type = '#{fact_name_class}'").index_by(&:name)
@db_facts ||= host.fact_values.eager_load(:fact_name).where("fact_names.type = '#{fact_name_class}'").index_by(&:name)
end
end

0 comments on commit 5aba0d1

Please sign in to comment.