From 6971ccbeca86576f1fc866fe81381de1510195d9 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Fri, 27 Apr 2018 10:47:55 +0200 Subject: [PATCH 1/6] Cleanup Persister serialization interface Cleanup Persister serialization interface. Renaming to to_hash/from_hash methods. Deleting YAML serialization for now, since it's way slower and we have problems that it keeps Symbols around. --- .../manager_refresh/inventory/persister.rb | 52 ++++++------------- 1 file changed, 15 insertions(+), 37 deletions(-) diff --git a/app/models/manager_refresh/inventory/persister.rb b/app/models/manager_refresh/inventory/persister.rb index b135172ae3c..9c859929887 100644 --- a/app/models/manager_refresh/inventory/persister.rb +++ b/app/models/manager_refresh/inventory/persister.rb @@ -36,27 +36,13 @@ def self.supported_collections # @param json_data [String] input JSON data # @return [ManagerRefresh::Inventory::Persister] Persister object loaded from a passed JSON def self.from_json(json_data) - from_raw_data(JSON.parse(json_data)) + from_hash(JSON.parse(json_data)) end # Returns serialized Persisted object to JSON # @return [String] serialized Persisted object to JSON def to_json - JSON.dump(to_raw_data) - end - - # Returns Persister object loaded from a passed YAML - # - # @param json_data [String] input JSON data - # @return [ManagerRefresh::Inventory::Persister] Persister object loaded from a passed YAML - def self.from_yaml(yaml_data) - from_raw_data(YAML.safe_load(yaml_data)) - end - - # Returns serialized Persisted object to YAML - # @return [String] serialized Persisted object to YAML - def to_yaml - YAML.dump(to_raw_data) + JSON.dump(to_hash) end # Creates method on class that lazy initializes an InventoryCollection @@ -186,16 +172,11 @@ def add_remaining_inventory_collections(defaults, options = {}) end # @return [Hash] entire Persister object serialized to hash - def to_raw_data - collections_data = collections.map do |key, collection| - next if collection.data.blank? && collection.manager_uuids.blank? && collection.all_manager_uuids.nil? - - { - :name => key, - :manager_uuids => collection.manager_uuids, - :all_manager_uuids => collection.all_manager_uuids, - :data => collection.to_raw_data - } + def to_hash + collections_data = collections.map do |_, collection| + next if collection.data.blank? && collection.targeted_scope.blank? && collection.all_manager_uuids.nil? + + collection.to_hash end.compact { @@ -212,27 +193,24 @@ class << self # # @param persister_data [Hash] serialized Persister object in hash # @return [ManagerRefresh::Inventory::Persister] Persister object built from serialized data - def from_raw_data(persister_data) - persister_data.transform_keys!(&:to_s) - + def from_hash(persister_data) # Extract the specific Persister class persister_class = persister_data['class'].constantize unless persister_class < ManagerRefresh::Inventory::Persister raise "Persister class must inherit from a ManagerRefresh::Inventory::Persister" end - # TODO(lsmola) do we need a target in this case? - # Load the Persister object and fill the InventoryCollections with the data - persister = persister_class.new(ManageIQ::Providers::BaseManager.find(persister_data['ems_id'])) - persister_data['collections'].each do |collection| - collection.transform_keys!(&:to_s) + ems = ManageIQ::Providers::BaseManager.find(persister_data['ems_id']) + persister = persister_class.new( + ems, + ManagerRefresh::TargetCollection.new(:manager => ems) # TODO(lsmola) we need to pass serialized targeted scope here + ) + persister_data['collections'].each do |collection| inventory_collection = persister.collections[collection['name'].try(:to_sym)] raise "Unrecognized InventoryCollection name: #{inventory_collection}" if inventory_collection.blank? - inventory_collection.manager_uuids.merge(collection['manager_uuids'] || []) - inventory_collection.all_manager_uuids = collection['all_manager_uuids'] - inventory_collection.from_raw_data(collection['data'], persister.collections) + inventory_collection.from_hash(collection, persister.collections) end persister end From a531cdf91daed2c75744b385a7ebad7ee766b81d Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Fri, 27 Apr 2018 10:53:29 +0200 Subject: [PATCH 2/6] Move IC serialization to it's own class Move IC serialization to it's own class, this way it's more concise what everything we need to do to serialize InventoryCollection, plus we can safely guard the recursively nested lazy links. --- .../manager_refresh/inventory_collection.rb | 5 +- .../inventory_collection/data_storage.rb | 60 +------- .../inventory_collection/reference.rb | 14 -- .../inventory_collection/serialization.rb | 130 ++++++++++++++++++ .../manager_refresh/inventory_object.rb | 9 -- .../manager_refresh/inventory_object_lazy.rb | 11 -- 6 files changed, 138 insertions(+), 91 deletions(-) create mode 100644 app/models/manager_refresh/inventory_collection/serialization.rb diff --git a/app/models/manager_refresh/inventory_collection.rb b/app/models/manager_refresh/inventory_collection.rb index 203fcdfa084..c5cfd34eb84 100644 --- a/app/models/manager_refresh/inventory_collection.rb +++ b/app/models/manager_refresh/inventory_collection.rb @@ -86,13 +86,12 @@ class InventoryCollection :each, :find_or_build, :find_or_build_by, - :from_raw_data, - :from_raw_value, + :from_hash, :index_proxy, :push, :size, :to_a, - :to_raw_data, + :to_hash, :to => :data_storage delegate :add_reference, diff --git a/app/models/manager_refresh/inventory_collection/data_storage.rb b/app/models/manager_refresh/inventory_collection/data_storage.rb index eab9c58d2d1..dd2ef841bd0 100644 --- a/app/models/manager_refresh/inventory_collection/data_storage.rb +++ b/app/models/manager_refresh/inventory_collection/data_storage.rb @@ -112,62 +112,14 @@ def to_a data end - # Reconstructs InventoryCollection from it's serialized form - # - # @param inventory_objects_data [Array[Hash]] Serialized array of InventoryObject objects as hashes - # @param available_inventory_collections [Array] List of available - # InventoryCollection objects - def from_raw_data(inventory_objects_data, available_inventory_collections) - inventory_objects_data.each do |inventory_object_data| - hash = inventory_object_data.each_with_object({}) do |(key, value), result| - result[key.to_sym] = if value.kind_of?(Array) - value.map { |x| from_raw_value(x, available_inventory_collections) } - else - from_raw_value(value, available_inventory_collections) - end - end - build(hash) - end - end - - # Transform serialized references into lazy InventoryObject objects - # - # @param value [Object, Hash] Serialized InventoryObject into Hash - # @param available_inventory_collections [Array] List of available - # InventoryCollection objects - # @return [Object, ManagerRefresh::InventoryObjectLazy] Returns ManagerRefresh::InventoryObjectLazy object - # if the serialized form was a reference, or return original value - def from_raw_value(value, available_inventory_collections) - if value.kind_of?(Hash) && (value['type'] || value[:type]) == "ManagerRefresh::InventoryObjectLazy" - value.transform_keys!(&:to_s) - end - - if value.kind_of?(Hash) && value['type'] == "ManagerRefresh::InventoryObjectLazy" - inventory_collection = available_inventory_collections[value['inventory_collection_name'].try(:to_sym)] - raise "Couldn't build lazy_link #{value} the inventory_collection_name was not found" if inventory_collection.blank? - inventory_collection.lazy_find(value['ems_ref'], :key => value['key'], :default => value['default']) - else - value - end + def to_hash + ManagerRefresh::InventoryCollection::Serialization.new(inventory_collection).to_hash end - # Serializes InventoryCollection's data storage into Array of Hashes, which we can turn into JSON or YAML - # - # @return [Array] Serialized InventoryCollection's data storage - def to_raw_data - data.map do |inventory_object| - inventory_object.data.transform_values do |value| - if inventory_object_lazy?(value) - value.to_raw_lazy_relation - elsif value.kind_of?(Array) && (inventory_object_lazy?(value.compact.first) || inventory_object?(value.compact.first)) - value.compact.map(&:to_raw_lazy_relation) - elsif inventory_object?(value) - value.to_raw_lazy_relation - else - value - end - end - end + def from_hash(inventory_objects_data, available_inventory_collections) + ManagerRefresh::InventoryCollection::Serialization + .new(inventory_collection) + .from_hash(inventory_objects_data, available_inventory_collections) end private diff --git a/app/models/manager_refresh/inventory_collection/reference.rb b/app/models/manager_refresh/inventory_collection/reference.rb index 1ee4d645f1e..44a17ae5e37 100644 --- a/app/models/manager_refresh/inventory_collection/reference.rb +++ b/app/models/manager_refresh/inventory_collection/reference.rb @@ -26,20 +26,6 @@ def primary? ref == :manager_ref end - # Returns serialized self into Hash - # - # @return [Hash] Serialized self into Hash - def to_hash - end - - class << self - # Returns reference object built from serialized Hash - # - # @return [ManagerRefresh::InventoryCollection::Reference] Reference object built from serialized Hash - def from_hash - end - end - class << self # Builds string uuid from passed Hash and keys # diff --git a/app/models/manager_refresh/inventory_collection/serialization.rb b/app/models/manager_refresh/inventory_collection/serialization.rb new file mode 100644 index 00000000000..372d678e510 --- /dev/null +++ b/app/models/manager_refresh/inventory_collection/serialization.rb @@ -0,0 +1,130 @@ +module ManagerRefresh + class InventoryCollection + class Serialization + delegate :all_manager_uuids, + :build, + :targeted_scope, + :data, + :inventory_object_lazy?, + :inventory_object?, + :name, + :to => :inventory_collection + + attr_reader :inventory_collection + + # @param inventory_collection [ManagerRefresh::InventoryCollection] InventoryCollection object we want the storage + # for + def initialize(inventory_collection) + @inventory_collection = inventory_collection + end + + # Loads InventoryCollection data from it's serialized form into existing InventoryCollection object + # + # @param inventory_objects_data [Hash] Serialized InventoryCollection as Hash + # @param available_inventory_collections [Array] List of available + # InventoryCollection objects + def from_hash(inventory_objects_data, available_inventory_collections) + inventory_objects_data['data'].each do |inventory_object_data| + build(hash_to_data(inventory_object_data, available_inventory_collections).symbolize_keys!) + end + + # TODO(lsmola) we need to remodel the targeted scope, to be able to serialize targeted InventoryCollections + # self.targeted_scope.merge!(inventory_objects_data['manager_uuids'] || []) + # self.all_manager_uuids = inventory_objects_data['all_manager_uuids'] + end + + # Serializes InventoryCollection's data storage into Array of Hashes + # + # @return [Hash] Serialized InventoryCollection object into Hash + def to_hash + { + :name => name, + :manager_uuids => targeted_scope.values.map { |x| data_to_hash(x) }, + :all_manager_uuids => all_manager_uuids, + :data => data.map { |x| data_to_hash(x.data) } + } + end + + private + + # Converts ManagerRefresh::InventoryObject or ManagerRefresh::InventoryObjectLazy into Hash + # + # @param value [ManagerRefresh::InventoryObject, ManagerRefresh::InventoryObjectLazy] InventoryObject or a lazy link + # @param depth [Integer] Depth of nesting for nested lazy link + # @return [Hash] Serialized ManagerRefresh::InventoryObjectLazy + def lazy_relation_to_hash(value, depth = 0) + { + :type => "ManagerRefresh::InventoryObjectLazy", + :inventory_collection_name => value.inventory_collection.name, + :reference => data_to_hash(value.reference.full_reference, depth + 1), + :ref => value.reference.ref, + :key => value.try(:key), + :default => value.try(:default), + } + end + + # Converts Hash to ManagerRefresh::InventoryObjectLazy + # + # @param value [Hash] Serialized InventoryObject or a lazy link + # @param available_inventory_collections [Array] List of available + # InventoryCollection objects + # @param depth [Integer] Depth of nesting for nested lazy link + # @return [ManagerRefresh::InventoryObjectLazy] Lazy link created from hash + def hash_to_lazy_relation(value, available_inventory_collections, depth = 0) + inventory_collection = available_inventory_collections[value['inventory_collection_name'].try(:to_sym)] + raise "Couldn't build lazy_link #{value} the inventory_collection_name was not found" if inventory_collection.blank? + + inventory_collection.lazy_find( + hash_to_data(value['reference'], available_inventory_collections, depth + 1).symbolize_keys!, + :ref => value['ref'].try(:to_sym), + :key => value['key'].try(:to_sym), + :default => value['default'] + ) + end + + # Converts Hash to attributes usable for building InventoryObject + # + # @param hash [Hash] Serialized InventoryObject data + # @param available_inventory_collections [Array] List of available + # InventoryCollection objects + # @param depth [Integer] Depth of nesting for nested lazy link + # @return [Hash] Hash with data usable for building InventoryObject + def hash_to_data(hash, available_inventory_collections, depth = 0) + raise "Nested lazy_relation of #{inventory_collection} is too deep, left processing: #{hash}" if depth > 20 + + hash.transform_values do |value| + if value.kind_of?(Hash) && value['type'] == "ManagerRefresh::InventoryObjectLazy" + hash_to_lazy_relation(value, available_inventory_collections, depth) + elsif value.kind_of?(Array) && value.first.kind_of?(Hash) && value.first['type'] == "ManagerRefresh::InventoryObjectLazy" + # TODO(lsmola) do we need to compact it sooner? What if first element is nil? On the other hand, we want to + # deprecate Vmthis HABTM assignment because it's not effective + value.compact.map { |x| hash_to_lazy_relation(x, available_inventory_collections, depth) } + else + value + end + end + end + + # Transforms data of the InventoryObject or Reference to InventoryObject into Hash + # + # @param data [Hash] Data of the InventoryObject or Reference to InventoryObject + # @param depth [Integer] Depth of nesting for nested lazy link + # @return [Hash] Serialized InventoryObject or Reference data into Hash + def data_to_hash(data, depth = 0) + raise "Nested lazy_relation of #{inventory_collection} is too deep, left processing: #{data}" if depth > 20 + + data.transform_values do |value| + if inventory_object_lazy?(value) + lazy_relation_to_hash(value, depth) + elsif value.kind_of?(Array) && (inventory_object_lazy?(value.compact.first) || inventory_object?(value.compact.first)) + value.compact.map { |x| lazy_relation_to_hash(x, depth) } + elsif inventory_object?(value) + lazy_relation_to_hash(value, depth) + else + value + end + end + end + end + end +end diff --git a/app/models/manager_refresh/inventory_object.rb b/app/models/manager_refresh/inventory_object.rb index 6d6797ff6fb..fd355859cf8 100644 --- a/app/models/manager_refresh/inventory_object.rb +++ b/app/models/manager_refresh/inventory_object.rb @@ -22,15 +22,6 @@ def manager_uuid reference.stringified_reference end - # @return [Hash] serialized InventoryObject into Lazy format - def to_raw_lazy_relation - { - :type => "ManagerRefresh::InventoryObjectLazy", - :inventory_collection_name => inventory_collection.name, - :ems_ref => manager_uuid, - } - end - # @return [ManagerRefresh::InventoryObject] returns self def load self diff --git a/app/models/manager_refresh/inventory_object_lazy.rb b/app/models/manager_refresh/inventory_object_lazy.rb index 072fcc8fe18..a6bf31a7e39 100644 --- a/app/models/manager_refresh/inventory_object_lazy.rb +++ b/app/models/manager_refresh/inventory_object_lazy.rb @@ -37,17 +37,6 @@ def inspect "InventoryObjectLazy:('#{self}', #{inventory_collection}#{suffix})" end - # @return [Hash] serialized InventoryObjectLazy - def to_raw_lazy_relation - { - :type => "ManagerRefresh::InventoryObjectLazy", - :inventory_collection_name => inventory_collection.name, - :reference => reference.to_hash, - :key => key, - :default => default, - } - end - # @return [ManagerRefresh::InventoryObject, Object] ManagerRefresh::InventoryObject instance or an attribute # on key def load From c27e9591ee5e69faa85bd527ded027b72da1df12 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Fri, 27 Apr 2018 11:09:57 +0200 Subject: [PATCH 3/6] Assert that reference exists before using it Assert that reference exists before using it --- .../inventory_collection/index/proxy.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/app/models/manager_refresh/inventory_collection/index/proxy.rb b/app/models/manager_refresh/inventory_collection/index/proxy.rb index 4375f272610..4bfdd2e816e 100644 --- a/app/models/manager_refresh/inventory_collection/index/proxy.rb +++ b/app/models/manager_refresh/inventory_collection/index/proxy.rb @@ -177,6 +177,10 @@ def assert_relation_keys(data, ref) end end + def assert_index_exists(ref) + raise "Index :#{ref} doesn't exist on #{inventory_collection}" if named_ref(ref).nil? + end + def assert_index(manager_uuid, ref) # TODO(lsmola) do we need some production logging too? Maybe the refresh log level could drive this # Let' do this really slick development and test env, but disable for production, since the checks are pretty @@ -186,6 +190,9 @@ def assert_index(manager_uuid, ref) if manager_uuid.kind_of?(ManagerRefresh::InventoryCollection::Reference) # ManagerRefresh::InventoryCollection::Reference has been already asserted, skip elsif manager_uuid.kind_of?(Hash) + # Test te index exists + assert_index_exists(ref) + # Test we are sending all keys required for the index unless required_index_keys_present?(manager_uuid.keys, ref) raise "Finder has missing keys for index :#{ref}, missing indexes are: #{missing_keys(manager_uuid.keys, ref)}" @@ -193,6 +200,9 @@ def assert_index(manager_uuid, ref) # Test that keys, that are relations, are nil or InventoryObject or InventoryObjectlazy class assert_relation_keys(manager_uuid, ref) else + # Test te index exists + assert_index_exists(ref) + # Check that other value (possibly String or Integer)) has no composite index if named_ref(ref).size > 1 right_format = "collection.find(#{named_ref(ref).map { |x| ":#{x} => 'X'" }.join(", ")}" From b4155b216c2269556aaf1c5c48a4e10383f69f48 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Fri, 27 Apr 2018 11:10:33 +0200 Subject: [PATCH 4/6] Correct the wrong type in YARD doc Correct the wrong type in YARD doc --- .../manager_refresh/inventory_collection/references_storage.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/manager_refresh/inventory_collection/references_storage.rb b/app/models/manager_refresh/inventory_collection/references_storage.rb index dd20c691694..26cdd084228 100644 --- a/app/models/manager_refresh/inventory_collection/references_storage.rb +++ b/app/models/manager_refresh/inventory_collection/references_storage.rb @@ -1,7 +1,7 @@ module ManagerRefresh class InventoryCollection class ReferencesStorage - # @return [Set] A set of InventoryObjects manager_uuids, which tells us which InventoryObjects were + # @return [Hash] A set of InventoryObjects manager_uuids, which tells us which InventoryObjects were # referenced by other InventoryObjects using a lazy_find. attr_reader :references From e0efeabe1cfa474b4e453a8ad82d1e0c3e8cd6c1 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Fri, 27 Apr 2018 11:11:02 +0200 Subject: [PATCH 5/6] Add basic specs for persister serialization Add basic specs for persister serialization --- .../helpers/spec_parsed_data.rb | 71 +++++----- .../persister/serializing_spec.rb | 124 ++++++++++++++++++ .../persister/test_persister.rb | 4 +- .../single_inventory_collection_spec.rb | 4 +- 4 files changed, 164 insertions(+), 39 deletions(-) create mode 100644 spec/models/manager_refresh/persister/serializing_spec.rb diff --git a/spec/models/manager_refresh/helpers/spec_parsed_data.rb b/spec/models/manager_refresh/helpers/spec_parsed_data.rb index 61560855004..aa58469b882 100644 --- a/spec/models/manager_refresh/helpers/spec_parsed_data.rb +++ b/spec/models/manager_refresh/helpers/spec_parsed_data.rb @@ -3,37 +3,38 @@ module SpecParsedData def vm_data(i, data = {}) { - :type => ManageIQ::Providers::CloudManager::Vm.name, - :ext_management_system => @ems, - :uid_ems => "vm_uid_ems_#{i}", - :ems_ref => "vm_ems_ref_#{i}", - :name => "vm_name_#{i}", - :vendor => "amazon", - :raw_power_state => "unknown", - :location => "vm_location_#{i}", + :type => ManageIQ::Providers::CloudManager::Vm.name, + :ems_id => @ems.id, + :uid_ems => "vm_uid_ems_#{i}", + :ems_ref => "vm_ems_ref_#{i}", + :name => "vm_name_#{i}", + :vendor => "amazon", + :raw_power_state => "unknown", + :location => "vm_location_#{i}", }.merge(data) end def key_pair_data(i, data = {}) { - :type => ManageIQ::Providers::CloudManager::AuthKeyPair.name, - :resource => @ems, - :name => "key_pair_name_#{i}", + :type => ManageIQ::Providers::CloudManager::AuthKeyPair.name, + :resource_id => @ems.id, + :resource_type => "ExtManagementSystem", + :name => "key_pair_name_#{i}", }.merge(data) end def image_data(i, data = {}) { - :type => ManageIQ::Providers::CloudManager::Template.name, - :ext_management_system => @ems, - :uid_ems => "image_uid_ems_#{i}", - :ems_ref => "image_ems_ref_#{i}", - :name => "image_name_#{i}", - :location => "image_location_#{i}", - :vendor => "amazon", - :raw_power_state => "never", - :template => true, - :publicly_available => false, + :type => ManageIQ::Providers::CloudManager::Template.name, + :ems_id => @ems.id, + :uid_ems => "image_uid_ems_#{i}", + :ems_ref => "image_ems_ref_#{i}", + :name => "image_name_#{i}", + :location => "image_location_#{i}", + :vendor => "amazon", + :raw_power_state => "never", + :template => true, + :publicly_available => false, }.merge(data) end @@ -67,20 +68,20 @@ def public_network_data(i, data = {}) def flavor_data(i, data = {}) { - :name => "t#{i}.nano", - :ext_management_system => @ems, + :name => "t#{i}.nano", + :ems_id => @ems.id, }.merge(data) end def orchestration_stack_data(i, data = {}) { - :ems_ref => "stack_ems_ref_#{i}", - :type => "ManageIQ::Providers::CloudManager::OrchestrationStack", - :ext_management_system => @ems, - :name => "stack_name_#{i}", - :description => "stack_description_#{i}", - :status => "stack_status_#{i}", - :status_reason => "stack_status_reason_#{i}", + :ems_ref => "stack_ems_ref_#{i}", + :type => "ManageIQ::Providers::CloudManager::OrchestrationStack", + :ems_id => @ems.id, + :name => "stack_name_#{i}", + :description => "stack_description_#{i}", + :status => "stack_status_#{i}", + :status_reason => "stack_status_reason_#{i}", }.merge(data) end @@ -95,11 +96,11 @@ def orchestration_stack_resource_data(i, data = {}) def network_port_data(i, data = {}) { - :name => "network_port_name_#{i}", - :ext_management_system => @ems.network_manager, - :ems_ref => "network_port_ems_ref_#{i}", - :status => "network_port_status#{i}", - :mac_address => "network_port_mac_#{i}", + :name => "network_port_name_#{i}", + :ems_id => @ems.network_manager.try(:id), + :ems_ref => "network_port_ems_ref_#{i}", + :status => "network_port_status#{i}", + :mac_address => "network_port_mac_#{i}", }.merge(data) end diff --git a/spec/models/manager_refresh/persister/serializing_spec.rb b/spec/models/manager_refresh/persister/serializing_spec.rb new file mode 100644 index 00000000000..a6c24c5bade --- /dev/null +++ b/spec/models/manager_refresh/persister/serializing_spec.rb @@ -0,0 +1,124 @@ +require_relative '../helpers/spec_parsed_data' +require_relative 'test_persister' +require_relative 'targeted_refresh_spec_helper' + +describe ManagerRefresh::Inventory::Persister do + include SpecParsedData + include TargetedRefreshSpecHelper + + ###################################################################################################################### + # Spec scenarios testing Perister can serialize/deserialize, with having complex nested lazy_find links + ###################################################################################################################### + # + before :each do + @zone = FactoryGirl.create(:zone) + @ems = FactoryGirl.create(:ems_cloud, + :zone => @zone, + :network_manager => FactoryGirl.create(:ems_network, :zone => @zone)) + + allow(@ems.class).to receive(:ems_type).and_return(:mock) + allow(Settings.ems_refresh).to receive(:mock).and_return({}) + end + + let(:persister) { create_persister } + + it "tests we can serialize inventory object with nested lazy references" do + persister = create_persister + populate_test_data(persister) + + ManagerRefresh::Inventory::Persister.from_json(persister.to_json).persist! + + counts = { + :disk => 2, + :flavor => 0, + :hardware => 3, + :miq_template => 1, + :network => 2, + :vm => 2, + :vm_or_template => 3 + } + + assert_counts(counts) + + vm = Vm.find_by(:ems_ref => "vm_ems_ref_1") + expect(vm.location).to eq("host_10_10_10_1.com") + expect(vm.hardware.guest_os).to eq("linux_generic_1") + expect(vm.hardware.guest_os_full_name).to eq("amazon") + expect(vm.hardware.networks.first.ipaddress).to eq("10.10.10.1") + expect(vm.hardware.disks.first.device_name).to eq("disk_name_1") + + vm2 = Vm.find_by(:ems_ref => "vm_ems_ref_2") + expect(vm2.hardware.networks.first.ipaddress).to eq("10.10.10.2") + expect(vm2.hardware.disks.first.device_name).to eq("disk_name_2") + + # TODO(lsmola) interestingly persister.hardwares.lazy_find({:vm_or_template => lazy_find_image_sec}) will not work + # because we build the reference only from the passed data. To make it work, we would have to do internally + # persister.hardwares.lazy_find({:vm_or_template => lazy_find_image_sec.load}), which will allow the nested lazy + # find to reach its :manager_ref data. That would mean the stringified reference would have to be build when + # we try to evaluate the lazy object. We should be able to do that. + expect(vm.hardware.model).to eq(nil) + expect(vm.hardware.manufacturer).to eq(nil) + end + + def populate_test_data(persister) + @image_data_1 = image_data(1) + @image_hardware_data_1 = image_hardware_data(1).merge( + :guest_os => "linux_generic_1", + :vm_or_template => persister.miq_templates.lazy_find(:ems_ref => image_data(1)[:ems_ref]), + :model => "test1", + :manufacturer => "test2" + ) + + # Nested lazy find + lazy_find_image = persister.miq_templates.lazy_find(:ems_ref => image_data(1)[:ems_ref]) + lazy_find_image_sec = persister.miq_templates.lazy_find({:name => image_data(1)[:name]}, {:ref => :by_name}) + lazy_find_image_sec1 = persister.miq_templates.lazy_find( + {:name => image_data(1)[:name], :uid_ems => image_data(1)[:uid_ems]}, {:ref => :by_uid_ems_and_name} + ) + lazy_find_image_sec2 = persister.miq_templates.lazy_find( + {:name => image_data(1)[:name], :uid_ems => image_data(1)[:uid_ems]}, {:ref => :by_uid_ems_and_name, :key => :vendor} + ) + lazy_find_vm = persister.vms.lazy_find(:ems_ref => vm_data(1)[:ems_ref]) + lazy_find_hardware = persister.hardwares.lazy_find(:vm_or_template => lazy_find_vm) + + @vm_data_1 = vm_data(1).merge( + :flavor => persister.flavors.lazy_find(:ems_ref => flavor_data(1)[:name]), + :genealogy_parent => persister.miq_templates.lazy_find(:ems_ref => image_data(1)[:ems_ref]), + :key_pairs => [persister.key_pairs.lazy_find(:name => key_pair_data(1)[:name])], + :location => persister.networks.lazy_find( + {:hardware => lazy_find_hardware, :description => "public"}, + {:key => :hostname, + :default => 'default_value_unknown'} + ), + ) + + @hardware_data_1 = hardware_data(1).merge( + :guest_os => persister.hardwares.lazy_find({:vm_or_template => lazy_find_image}, {:key => :guest_os}), + :model => persister.hardwares.lazy_find({:vm_or_template => lazy_find_image_sec}, {:key => :model}), + :manufacturer => persister.hardwares.lazy_find({:vm_or_template => lazy_find_image_sec1}, {:key => :manufacturer}), + :guest_os_full_name => lazy_find_image_sec2, + :vm_or_template => persister.vms.lazy_find(:ems_ref => vm_data(1)[:ems_ref]) + ) + + @public_network_data_1 = public_network_data(1).merge( + :hardware => persister.hardwares.lazy_find(:vm_or_template => lazy_find_vm), + ) + + @disk_data_1 = disk_data(1).merge( + :hardware => persister.hardwares.lazy_find(:vm_or_template => lazy_find_vm), + ) + + persister.miq_templates.build(@image_data_1) + persister.hardwares.build(@image_hardware_data_1) + persister.vms.build(@vm_data_1) + persister.hardwares.build(@hardware_data_1) + persister.networks.build(@public_network_data_1) + persister.disks.build(@disk_data_1) + + # Try also data with relations without lazy link + vm = persister.vms.build(vm_data(2)) + hardware = persister.hardwares.build(hardware_data(2).merge(:vm_or_template => vm)) + persister.networks.build(public_network_data(2).merge(:hardware => hardware)) + persister.disks.build(disk_data(2).merge(:hardware => hardware)) + end +end diff --git a/spec/models/manager_refresh/persister/test_persister.rb b/spec/models/manager_refresh/persister/test_persister.rb index 82f47fe45c5..96e8c1f0996 100644 --- a/spec/models/manager_refresh/persister/test_persister.rb +++ b/spec/models/manager_refresh/persister/test_persister.rb @@ -4,12 +4,12 @@ def initialize_inventory_collections # Top level models with direct references for Cloud add_inventory_collections_with_references( cloud, - %i(vms), + %i(vms miq_templates), :secondary_refs => {:by_name => [:name], :by_uid_ems_and_name => %i(uid_ems name)} ) add_inventory_collections_with_references( cloud, - %i(miq_templates availability_zones orchestration_stacks) + %i(availability_zones orchestration_stacks) ) add_inventory_collection_with_references( diff --git a/spec/models/manager_refresh/save_inventory/single_inventory_collection_spec.rb b/spec/models/manager_refresh/save_inventory/single_inventory_collection_spec.rb index 180fa3f62bd..d8f0a9a6a97 100644 --- a/spec/models/manager_refresh/save_inventory/single_inventory_collection_spec.rb +++ b/spec/models/manager_refresh/save_inventory/single_inventory_collection_spec.rb @@ -452,7 +452,7 @@ :parent => @ems, :association => :vms, # TODO(lsmola) vendor is not getting caught by fixed attributes - :attributes_whitelist => [:uid_ems, :vendor, :ext_management_system] + :attributes_whitelist => [:uid_ems, :vendor, :ext_management_system, :ems_id] ) # Fill the InventoryCollections with data, that have a modified name, new VM and a missing VM @@ -493,7 +493,7 @@ :parent => @ems, :association => :vms, # TODO(lsmola) vendor is not getting caught by fixed attributes - :attributes_whitelist => [:uid_ems, :raw_power_state, :vendor, :ext_management_system], + :attributes_whitelist => [:uid_ems, :raw_power_state, :vendor, :ems_id, :ext_management_system], :attributes_blacklist => [:name, :ems_ref, :raw_power_state] ) From b96e4ce081594d03a6a276961a8a12e2f4864ea3 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Wed, 2 May 2018 08:07:20 +0200 Subject: [PATCH 6/6] Remove unused :let and typo in spec Remove unused :let and typo in spec --- spec/models/manager_refresh/persister/serializing_spec.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/spec/models/manager_refresh/persister/serializing_spec.rb b/spec/models/manager_refresh/persister/serializing_spec.rb index a6c24c5bade..aa7e5dd7f83 100644 --- a/spec/models/manager_refresh/persister/serializing_spec.rb +++ b/spec/models/manager_refresh/persister/serializing_spec.rb @@ -7,7 +7,7 @@ include TargetedRefreshSpecHelper ###################################################################################################################### - # Spec scenarios testing Perister can serialize/deserialize, with having complex nested lazy_find links + # Spec scenarios testing Persister can serialize/deserialize, with having complex nested lazy_find links ###################################################################################################################### # before :each do @@ -20,8 +20,6 @@ allow(Settings.ems_refresh).to receive(:mock).and_return({}) end - let(:persister) { create_persister } - it "tests we can serialize inventory object with nested lazy references" do persister = create_persister populate_test_data(persister)