diff --git a/app/models/miq_provision_virt_workflow.rb b/app/models/miq_provision_virt_workflow.rb index ae9643595e1..068056e79bc 100644 --- a/app/models/miq_provision_virt_workflow.rb +++ b/app/models/miq_provision_virt_workflow.rb @@ -230,8 +230,6 @@ def available_vlans_and_hosts(options = {}) hosts = get_selected_hosts(src) unless @vlan_options[:vlans] == false rails_logger('allowed_vlans', 0) - # TODO: Use Active Record to preload this data? - MiqPreloader.preload(hosts, :lans => :switches) load_allowed_vlans(hosts, vlans) rails_logger('allowed_vlans', 1) end @@ -244,9 +242,11 @@ def load_allowed_vlans(hosts, vlans) end def load_hosts_vlans(hosts, vlans) - hosts.each do |h| - h.lans.each { |l| vlans[l.name] = l.name unless l.switch.shared? } - end + Lan.joins(:switch => :hosts) + .where(:hosts => {:id => hosts.map(&:id)}) + .where(:switches => {:shared => [nil, false]}) + .distinct.pluck(:name) + .each_with_object(vlans) { |v, h| h[v] = v } end def filter_by_tags(target, options) diff --git a/spec/models/miq_provision_virt_workflow_spec.rb b/spec/models/miq_provision_virt_workflow_spec.rb index 7a61830692e..230dfce52a6 100644 --- a/spec/models/miq_provision_virt_workflow_spec.rb +++ b/spec/models/miq_provision_virt_workflow_spec.rb @@ -67,9 +67,11 @@ context 'network selection' do before do - @ems = FactoryGirl.create(:ems_vmware) - @host1 = FactoryGirl.create(:host_vmware, :ems_id => @ems.id) - @src_vm = FactoryGirl.create(:vm_vmware, :host => @host1, :ems_id => @ems.id) + @ems = FactoryGirl.create(:ems_vmware) + @host1 = FactoryGirl.create(:host_vmware, :ems_id => @ems.id) + @host2 = FactoryGirl.create(:host_vmware, :ems_id => @ems.id) + @src_vm = FactoryGirl.create(:vm_vmware, :host => @host1, :ems_id => @ems.id) + @other_vm = FactoryGirl.create(:vm_vmware, :host => @host2, :ems_id => @ems.id) allow(Rbac).to receive(:search) do |hash| [Array.wrap(hash[:targets])] end @@ -84,24 +86,30 @@ s11 = FactoryGirl.create(:switch, :name => "A") s12 = FactoryGirl.create(:switch, :name => "B") s13 = FactoryGirl.create(:switch, :name => "C") - @src_vm.host.switches = [s11, s12, s13] + s14 = FactoryGirl.create(:switch, :name => "D") + @src_vm.host.switches = [s11, s12, s13] + @other_vm.host.switches = [s14] @lan11 = FactoryGirl.create(:lan, :name => "lan_A", :switch_id => s11.id) @lan12 = FactoryGirl.create(:lan, :name => "lan_B", :switch_id => s12.id) @lan13 = FactoryGirl.create(:lan, :name => "lan_C", :switch_id => s13.id) + @lan14 = FactoryGirl.create(:lan, :name => "lan_D", :switch_id => s14.id) end it '#allowed_vlans' do - allow(workflow).to receive(:allowed_hosts).with(no_args).and_return([workflow.host_to_hash_struct(@host1)]) + allowed_hosts = [ + workflow.host_to_hash_struct(@host1), + workflow.host_to_hash_struct(@host2) + ] + allow(workflow).to receive(:allowed_hosts).with(no_args).and_return(allowed_hosts) vlans = workflow.allowed_vlans(:vlans => true, :dvs => false) - lan_keys = [@lan11.name, @lan13.name, @lan12.name] + lan_keys = [@lan11.name, @lan13.name, @lan12.name, @lan14.name] expect(vlans.keys).to match_array(lan_keys) expect(vlans.values).to match_array(lan_keys) end it '#load_hosts_vlans' do - hosts = [@host1] - MiqPreloader.preload(hosts, :lans => :switches) - expect { workflow.load_hosts_vlans(hosts, {}) }.not_to exceed_query_limit(0) + hosts = [@host1, @host2] + expect { workflow.load_hosts_vlans(hosts, {}) }.not_to exceed_query_limit(1) end end end