Merge pull request #157 from saulotoledo/parser_strings_cleanup

Moving strings to dictionary and cleaning up empty and malformatted strings

app/models/manageiq/providers/lenovo/physical_infra_manager/parser/component_parser.rb

@@ -29,7 +29,7 @@ def self.parse(source, dictionary)
               source_value = source_value[source_key]
             end
           end
-          result[key] = source_value
+          result[key] = source_value.kind_of?(String) ? source_value.strip.presence : source_value
         elsif value.kind_of?(Hash)
           result[key] = parse(source, dictionary[key])
         end

app/models/manageiq/providers/lenovo/physical_infra_manager/parser/firmware_parser.rb

@@ -9,12 +9,11 @@ class << self
       # @return [Hash] the firmware as required by the application
       #
       def parse_firmware(firmware)
-        {
-          :name         => "#{firmware["role"]} #{firmware["name"]}-#{firmware["status"]}".strip,
-          :build        => firmware["build"].presence,
-          :version      => firmware["version"].presence,
-          :release_date => firmware["date"].presence,
-        }
+        result = parse(firmware, parent::ParserDictionaryConstants::FIRMWARE)
+
+        result[:name] = "#{firmware["role"]} #{firmware["name"]}-#{firmware["status"]}".strip
+
+        result
       end
     end
   end

app/models/manageiq/providers/lenovo/physical_infra_manager/parser/network_device_parser.rb

@@ -51,15 +51,15 @@ def network_device?(device)
       end
 
       def parse_device(device)
-        {
-          :uid_ems                => mount_uuid(device),
-          :device_name            => device["productName"] ? device["productName"] : device["name"],
-          :device_type            => "ethernet",
-          :firmwares              => parse_device_firmware(device),
-          :manufacturer           => device["manufacturer"],
-          :field_replaceable_unit => device["FRU"],
-          :location               => device['slotNumber'] ? "Bay #{device['slotNumber']}" : nil,
-        }
+        result = parse(device, parent::ParserDictionaryConstants::GUEST_DEVICE)
+
+        result[:uid_ems]     = mount_uuid(device)
+        result[:device_name] = device["productName"] ? device["productName"] : device["name"]
+        result[:device_type] = "ethernet"
+        result[:firmwares]   = parse_device_firmware(device)
+        result[:location]    = device['slotNumber'] ? "Bay #{device['slotNumber']}" : nil
+
+        result
       end
 
       def parse_device_firmware(device)

app/models/manageiq/providers/lenovo/physical_infra_manager/parser/parser_dictionary_constants.rb

@@ -6,7 +6,7 @@ class PhysicalInfraManager::Parser::ParserDictionaryConstants
       8  => "on",
       5  => "off",
       18 => "Standby",
-      0  => "Unknown"
+      0  => "Unknown",
     }.freeze
 
     HEALTH_STATE_MAP = {
@@ -19,7 +19,7 @@ class PhysicalInfraManager::Parser::ParserDictionaryConstants
       "major-failure"   => "Critical",
       "non-recoverable" => "Critical",
       "fatal"           => "Critical",
-      nil               => "Unknown"
+      nil               => "Unknown",
     }.freeze
 
     MIQ_TYPES = {
@@ -29,7 +29,7 @@ class PhysicalInfraManager::Parser::ParserDictionaryConstants
     }.freeze
 
     PROPERTIES_MAP = {
-      :led_identify_name => %w(Identification Identify)
+      :led_identify_name => %w(Identification Identify),
     }.freeze
 
     # TRANSLATE HASHES BEGIN
@@ -44,10 +44,12 @@ class PhysicalInfraManager::Parser::ParserDictionaryConstants
       :switch_uuid  => 'uuid',
       :power_state  => 'powerState',
       :asset_detail => {
-        :product_name  => 'productName',
-        :serial_number => 'serialNumber',
-        :description   => 'description',
-        :manufacturer  => 'manufacturer',
+        :product_name           => 'productName',
+        :serial_number          => 'serialNumber',
+        :part_number            => 'partNumber',
+        :field_replaceable_unit => 'FRU',
+        :description            => 'description',
+        :manufacturer           => 'manufacturer',
       },
     }.freeze
 
@@ -56,6 +58,11 @@ class PhysicalInfraManager::Parser::ParserDictionaryConstants
       :default_gateway => 'gateway',
     }.freeze
 
+    PHYSICAL_SWITCH_PORT = {
+      :peer_mac_address => 'peerMacAddress',
+      :vlan_key         => 'PVID',
+    }.freeze
+
     PHYSICAL_SERVER = {
       :name            => 'name',
       :ems_ref         => 'uuid',
@@ -74,12 +81,12 @@ class PhysicalInfraManager::Parser::ParserDictionaryConstants
         :location               => 'location.location',
         :room                   => 'location.room',
         :rack_name              => 'location.rack',
-        :lowest_rack_unit       => 'location.lowestRackUnit'
+        :lowest_rack_unit       => 'location.lowestRackUnit',
       },
       :computer_system => {
         :hardware => {
           :guest_devices => '',
-          :firmwares     => ''
+          :firmwares     => '',
         },
       },
     }.freeze
@@ -90,12 +97,31 @@ class PhysicalInfraManager::Parser::ParserDictionaryConstants
       :ems_ref => 'UUID',
     }.freeze
 
+    MANAGEMENT_DEVICE = {
+      :address => 'macAddress',
+      :network => {
+        :ipaddress => 'mgmtProcIPaddress',
+      },
+    }.freeze
+
+    GUEST_DEVICE = {
+      :manufacturer           => 'manufacturer',
+      :field_replaceable_unit => 'FRU',
+      :controller_type        => 'class',
+    }.freeze
+
+    FIRMWARE = {
+      :build        => 'build',
+      :version      => 'version',
+      :release_date => 'date',
+    }.freeze
+
     CONFIG_PATTERNS = {
       :manager_ref  => 'id',
       :name         => 'name',
       :description  => 'description',
       :user_defined => 'userDefined',
-      :in_use       => 'inUse'
+      :in_use       => 'inUse',
     }.freeze
     # TRANSLATE HASH END
   end

app/models/manageiq/providers/lenovo/physical_infra_manager/parser/physical_server_parser.rb

@@ -80,18 +80,12 @@ def get_guest_devices(node)
       end
 
       def parse_management_device(node)
-        {
-          :device_type => "management",
-          :network     => parse_management_network(node),
-          :address     => node.macAddress
-        }
-      end
+        result = parse(node, parent::ParserDictionaryConstants::MANAGEMENT_DEVICE)
 
-      def parse_management_network(node)
-        {
-          :ipaddress   => node.mgmtProcIPaddress,
-          :ipv6address => node.ipv6Addresses.nil? ? node.ipv6Addresses : node.ipv6Addresses.join(", ")
-        }
+        result[:device_type] = "management"
+        result[:network][:ipv6address] = node.ipv6Addresses.nil? ? node.ipv6Addresses : node.ipv6Addresses.join(", ")
+
+        result
       end
     end
   end

app/models/manageiq/providers/lenovo/physical_infra_manager/parser/physical_switch_parser.rb

@@ -18,9 +18,6 @@ def parse_physical_switch(physical_switch)
         result[:health_state] = parent::ParserDictionaryConstants::HEALTH_STATE_MAP[physical_switch.overallHealthState.nil? ? physical_switch.overallHealthState : physical_switch.overallHealthState.downcase]
         result[:hardware]     = get_hardwares(physical_switch)
 
-        result[:asset_detail][:part_number]            = physical_switch.partNumber.presence&.strip
-        result[:asset_detail][:field_replaceable_unit] = physical_switch.FRU.presence&.strip
-
         return physical_switch.uuid, result
       end
 
@@ -68,13 +65,13 @@ def parse_network(assignment, is_ipv6 = false)
       end
 
       def parse_port(port)
-        {
-          :device_name      => port["portName"].presence || port["port"],
-          :device_type      => "physical_port",
-          :peer_mac_address => port["peerMacAddress"].presence,
-          :vlan_key         => port["PVID"].presence,
-          :vlan_enabled     => port["PVID"].present?
-        }
+        result = parse(port, parent::ParserDictionaryConstants::PHYSICAL_SWITCH_PORT)
+
+        result[:device_name]  = port["portName"].presence || port["port"]
+        result[:device_type]  = "physical_port"
+        result[:vlan_enabled] = port["PVID"].present?
+
+        result
       end
 
       def get_firmwares(physical_switch)

app/models/manageiq/providers/lenovo/physical_infra_manager/parser/storage_device_parser.rb

@@ -20,16 +20,15 @@ def parse_storage_device(component)
       # @param [Hash] device - a hash containing the storage device informations
       #
       def parse_device(device)
-        {
-          :uid_ems                => mount_uuid(device),
-          :device_name            => device["productName"] ? device["productName"] : device["name"],
-          :device_type            => "storage",
-          :firmwares              => parse_device_firmware(device),
-          :manufacturer           => device["manufacturer"],
-          :field_replaceable_unit => device["FRU"],
-          :location               => device['slotNumber'] ? "Bay #{device['slotNumber']}" : nil,
-          :controller_type        => device["class"],
-        }
+        result = parse(device, parent::ParserDictionaryConstants::GUEST_DEVICE)
+
+        result[:uid_ems]     = mount_uuid(device)
+        result[:device_name] = device["productName"] ? device["productName"] : device["name"]
+        result[:device_type] = "storage"
+        result[:firmwares]   = parse_device_firmware(device)
+        result[:location]    = device['slotNumber'] ? "Bay #{device['slotNumber']}" : nil
+
+        result
       end
 
       #

spec/models/manageiq/providers/lenovo/physical_infra_manager/refresh_parser_spec.rb

@@ -127,18 +127,26 @@
       expect(storage_device[:location]).to eq("Bay 12")
     end
 
+    %i(
+      field_replaceable_unit
+      contact
+      location
+      room
+      rack_name
+    ).each do |attr|
+      it "will retrieve nil for #{attr} if parsed data is an empty string" do
+        asset_detail = @result[:physical_servers].first[:asset_detail]
+        expect(asset_detail[attr]).to be(nil)
+      end
+    end
+
     %i(
       product_name
       manufacturer
       machine_type
       model
       serial_number
-      field_replaceable_unit
-      contact
       description
-      location
-      room
-      rack_name
       lowest_rack_unit
     ).each do |attr|
       it "will retrieve #{attr} of asset detail" do