From 2c5ddadd09aa7debde6049bc23044ddfe9b1dd8a Mon Sep 17 00:00:00 2001 From: "Saulo S. Toledo" <saulotoledo@gmail.com> Date: Mon, 16 Apr 2018 17:37:49 -0300 Subject: [PATCH] Moving strings to dictionary and cleaning up empty and malformatted strings --- .../parser/component_parser.rb | 2 +- .../parser/firmware_parser.rb | 11 ++--- .../parser/network_device_parser.rb | 18 ++++---- .../parser/parser_dictionary_constants.rb | 46 +++++++++++++++---- .../parser/physical_server_parser.rb | 16 ++----- .../parser/physical_switch_parser.rb | 17 +++---- .../parser/storage_device_parser.rb | 19 ++++---- .../refresh_parser_spec.rb | 18 ++++++-- 8 files changed, 85 insertions(+), 62 deletions(-) diff --git a/app/models/manageiq/providers/lenovo/physical_infra_manager/parser/component_parser.rb b/app/models/manageiq/providers/lenovo/physical_infra_manager/parser/component_parser.rb index 720eb66bcb..d3a24810a1 100644 --- a/app/models/manageiq/providers/lenovo/physical_infra_manager/parser/component_parser.rb +++ b/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 diff --git a/app/models/manageiq/providers/lenovo/physical_infra_manager/parser/firmware_parser.rb b/app/models/manageiq/providers/lenovo/physical_infra_manager/parser/firmware_parser.rb index c0a4fd6784..e8bdf60e34 100644 --- a/app/models/manageiq/providers/lenovo/physical_infra_manager/parser/firmware_parser.rb +++ b/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 diff --git a/app/models/manageiq/providers/lenovo/physical_infra_manager/parser/network_device_parser.rb b/app/models/manageiq/providers/lenovo/physical_infra_manager/parser/network_device_parser.rb index f1e4543809..8b97be534d 100644 --- a/app/models/manageiq/providers/lenovo/physical_infra_manager/parser/network_device_parser.rb +++ b/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) diff --git a/app/models/manageiq/providers/lenovo/physical_infra_manager/parser/parser_dictionary_constants.rb b/app/models/manageiq/providers/lenovo/physical_infra_manager/parser/parser_dictionary_constants.rb index 45c6f68640..b14dbcb666 100644 --- a/app/models/manageiq/providers/lenovo/physical_infra_manager/parser/parser_dictionary_constants.rb +++ b/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', @@ -73,12 +80,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 @@ -89,12 +96,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 diff --git a/app/models/manageiq/providers/lenovo/physical_infra_manager/parser/physical_server_parser.rb b/app/models/manageiq/providers/lenovo/physical_infra_manager/parser/physical_server_parser.rb index aeaa9cbdbf..1ddabed013 100644 --- a/app/models/manageiq/providers/lenovo/physical_infra_manager/parser/physical_server_parser.rb +++ b/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 diff --git a/app/models/manageiq/providers/lenovo/physical_infra_manager/parser/physical_switch_parser.rb b/app/models/manageiq/providers/lenovo/physical_infra_manager/parser/physical_switch_parser.rb index 9cd03233ac..7024193a20 100644 --- a/app/models/manageiq/providers/lenovo/physical_infra_manager/parser/physical_switch_parser.rb +++ b/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) diff --git a/app/models/manageiq/providers/lenovo/physical_infra_manager/parser/storage_device_parser.rb b/app/models/manageiq/providers/lenovo/physical_infra_manager/parser/storage_device_parser.rb index e4459cf79d..8a26939925 100644 --- a/app/models/manageiq/providers/lenovo/physical_infra_manager/parser/storage_device_parser.rb +++ b/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 # diff --git a/spec/models/manageiq/providers/lenovo/physical_infra_manager/refresh_parser_spec.rb b/spec/models/manageiq/providers/lenovo/physical_infra_manager/refresh_parser_spec.rb index 95c6c5b421..0eefb5954d 100644 --- a/spec/models/manageiq/providers/lenovo/physical_infra_manager/refresh_parser_spec.rb +++ b/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