From 05664019f9acd1a97ff419085c217099194b433c Mon Sep 17 00:00:00 2001
From: Greg McCullough <gmccullo@redhat.com>
Date: Sat, 4 Nov 2017 19:47:35 -0400
Subject: [PATCH] Properly update network hash when the first array element is
 nil

Refactor and add tests for normalize_network_adapter_settings

https://bugzilla.redhat.com/show_bug.cgi?id=1508069#c4
---
 .../provision/configuration/network.rb        | 40 +++++++++-----
 .../provision/configuration/network_spec.rb   | 55 +++++++++++++++++++
 2 files changed, 81 insertions(+), 14 deletions(-)
 create mode 100644 spec/models/manageiq/providers/vmware/infra_manager/provision/configuration/network_spec.rb

diff --git a/app/models/manageiq/providers/vmware/infra_manager/provision/configuration/network.rb b/app/models/manageiq/providers/vmware/infra_manager/provision/configuration/network.rb
index 01b4ad745..40707fe68 100644
--- a/app/models/manageiq/providers/vmware/infra_manager/provision/configuration/network.rb
+++ b/app/models/manageiq/providers/vmware/infra_manager/provision/configuration/network.rb
@@ -28,21 +28,12 @@ def build_config_network_adapters(vmcs)
   end
 
   def normalize_network_adapter_settings
-    if options[:networks].blank?
-      vlan = get_option(:vlan)
-      _log.info("vlan: #{vlan.inspect}")
-      unless vlan.nil?
-        options[:networks] = [] << net = {:network => vlan, :mac_address => get_option_last(:mac_address)}
-        if vlan[0, 4] == 'dvs_'
-          # Remove the "dvs_" prefix on the name
-          net[:network] = vlan[4..-1]
-          net[:is_dvs]  = true
-        end
-      end
+    options[:networks] = Array(options[:networks])
+
+    if options[:networks].first.blank?
+      convert_vlan_options_to_network_hash
     else
-      # When using advanced network settings update the options hash to reflect the selected vlan
-      net = options[:networks].first
-      options[:vlan] = [net[:is_dvs] == true ? "dvs_#{net[:network]}" : net[:network], net[:network]]
+      convert_network_hash_to_vlan_options
     end
     options[:networks]
   end
@@ -172,4 +163,25 @@ def get_network_device(vimVm, _vmcs, _vim = nil, vlan = nil)
       end
     end
   end
+
+  private def convert_vlan_options_to_network_hash
+    vlan = get_option(:vlan)
+    _log.info("vlan: #{vlan.inspect}")
+    return unless vlan
+
+    options[:networks][0] = {:network => vlan}.tap do |net|
+      net[:mac_address] = get_option_last(:mac_address) if get_option_last(:mac_address)
+
+      if vlan[0, 4] == 'dvs_'
+        # Remove the "dvs_" prefix on the name
+        net[:network] = vlan[4..-1]
+        net[:is_dvs]  = true
+      end
+    end
+  end
+
+  private def convert_network_hash_to_vlan_options
+    net = options[:networks].first
+    options[:vlan] = [net[:is_dvs] == true ? "dvs_#{net[:network]}" : net[:network], net[:network]]
+  end
 end
diff --git a/spec/models/manageiq/providers/vmware/infra_manager/provision/configuration/network_spec.rb b/spec/models/manageiq/providers/vmware/infra_manager/provision/configuration/network_spec.rb
new file mode 100644
index 000000000..4ffdbf953
--- /dev/null
+++ b/spec/models/manageiq/providers/vmware/infra_manager/provision/configuration/network_spec.rb
@@ -0,0 +1,55 @@
+describe ManageIQ::Providers::Vmware::InfraManager::Provision::Configuration::Network do
+  describe '#normalize_network_adapter_settings' do
+    let(:miq_provision) { FactoryGirl.build(:miq_provision_vmware, :options => options) }
+
+    shared_examples_for 'normalize_network_adapter_settings' do
+      it 'updates network options' do
+        miq_provision.normalize_network_adapter_settings
+
+        expect(miq_provision.options).to include(network_options)
+      end
+    end
+
+    context 'adds default adapter into networks hash' do
+      let(:options) { {:vlan => %w(network network), :mac_address => 'aa:bb:cc:dd:ee:ff'} }
+      let(:network_options)  { {:networks=>[{:network => 'network', :mac_address => 'aa:bb:cc:dd:ee:ff'}]} }
+
+      it_behaves_like 'normalize_network_adapter_settings'
+    end
+
+    context 'adds default adapter into networks hash' do
+      let(:options) { {:vlan => %w(dvs_network network)} }
+      let(:network_options)  { {:networks => [{:network => 'network', :is_dvs => true}]} }
+
+      it_behaves_like 'normalize_network_adapter_settings'
+    end
+
+    context 'adds default adapter into networks hash' do
+      let(:options) { {:vlan => %w(network network), :networks => []} }
+      let(:network_options)  { {:networks=>[{:network=>'network'}]} }
+
+      it_behaves_like 'normalize_network_adapter_settings'
+    end
+
+    context 'adds default adapter into networks hash' do
+      let(:options) { {:vlan => %w(network network), :networks => [nil]} }
+      let(:network_options)  { {:networks=>[{:network=>'network'}]} }
+
+      it_behaves_like 'normalize_network_adapter_settings'
+    end
+
+    context 'adds default adapter into networks hash' do
+      let(:options) { {:networks => [{:network => 'network'}]} }
+      let(:network_options) { {:vlan => %w(network network)} }
+
+      it_behaves_like 'normalize_network_adapter_settings'
+    end
+
+    context 'adds default adapter into networks hash' do
+      let(:options) { {:networks => [{:network => 'network', :is_dvs => true}]} }
+      let(:network_options) { {:vlan => %w(dvs_network network)} }
+
+      it_behaves_like 'normalize_network_adapter_settings'
+    end
+  end
+end