From d9be0c21386bb94a827e6304cd7ab1de65db79bf Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Wed, 22 Feb 2017 14:43:23 +0100 Subject: [PATCH 01/18] We want to allow setting references as public We want to allow setting references as public --- app/models/manager_refresh/inventory_collection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/manager_refresh/inventory_collection.rb b/app/models/manager_refresh/inventory_collection.rb index 097f74b4b42..7c200810e76 100644 --- a/app/models/manager_refresh/inventory_collection.rb +++ b/app/models/manager_refresh/inventory_collection.rb @@ -335,7 +335,7 @@ def db_collection_for_comparison private - attr_writer :attributes_blacklist, :attributes_whitelist, :db_data_index, :references + attr_writer :attributes_blacklist, :attributes_whitelist, :db_data_index # Finds manager_uuid in the DB. Using a configured strategy we cache obtained data in the db_data_index, so the # same find will not hit database twice. Also if we use lazy_links and this is called when From 1834f74803b39d79eb88978c2e71b2fe00643335 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Wed, 22 Feb 2017 14:46:02 +0100 Subject: [PATCH 02/18] The constant is written as O(1) not O(C) The constant is written as O(1) not O(C) --- app/models/manager_refresh/inventory_collection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/manager_refresh/inventory_collection.rb b/app/models/manager_refresh/inventory_collection.rb index 7c200810e76..cc7ac5ab3ba 100644 --- a/app/models/manager_refresh/inventory_collection.rb +++ b/app/models/manager_refresh/inventory_collection.rb @@ -355,7 +355,7 @@ def find_in_db(manager_uuid) else return db_data_index[manager_uuid] if db_data_index && db_data_index[manager_uuid] # We haven't found the reference, lets add it to the list of references and load it - references << manager_uuid unless references.include?(manager_uuid) # O(C) since references is Set + references << manager_uuid unless references.include?(manager_uuid) # O(1) since references is Set end populate_db_data_index! From 6292eaa0809f4a27ed367afa8cdc3d52fb0bf1ee Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Wed, 22 Feb 2017 14:49:24 +0100 Subject: [PATCH 03/18] Calling .include? on Set is O(1) Calling .include? on Set is O(1) --- app/models/manager_refresh/inventory_object.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/models/manager_refresh/inventory_object.rb b/app/models/manager_refresh/inventory_object.rb index f6bf56b0084..be99845f85b 100644 --- a/app/models/manager_refresh/inventory_object.rb +++ b/app/models/manager_refresh/inventory_object.rb @@ -138,7 +138,6 @@ def allowed?(inventory_collection_scope, key) foreign_to_association = inventory_collection_scope.foreign_key_to_association_mapping[key] || inventory_collection_scope.foreign_type_to_association_mapping[key] - # TODO(lsmola) can we make this O(1)? This check will be performed for each record in the DB return false if inventory_collection_scope.attributes_blacklist.present? && (inventory_collection_scope.attributes_blacklist.include?(key) || (foreign_to_association && inventory_collection_scope.attributes_blacklist.include?(foreign_to_association))) From 424537629262a0579657b2e02f2f932501276d9c Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Wed, 22 Feb 2017 14:50:12 +0100 Subject: [PATCH 04/18] Not relevant comment, attributes is callable multiple times now Not relevant comment, attributes is callable multiple times now --- app/models/manager_refresh/inventory_object.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/models/manager_refresh/inventory_object.rb b/app/models/manager_refresh/inventory_object.rb index be99845f85b..b476cff84e0 100644 --- a/app/models/manager_refresh/inventory_object.rb +++ b/app/models/manager_refresh/inventory_object.rb @@ -23,10 +23,6 @@ def load end def attributes(inventory_collection_scope = nil) - # TODO(lsmola) mark method with !, for performance reasons, this methods can be called only once, the second - # call will not return saveable result. We do not want to cache the result, since we want the lowest memory - # footprint. - # We should explicitly pass a scope, since the inventory_object can be mapped to more InventoryCollections with # different blacklist and whitelist. The generic code always passes a scope. inventory_collection_scope ||= inventory_collection From 8e8a409f65c3fd22fe304f75fbd0f0e6489c1747 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Wed, 22 Feb 2017 18:36:26 +0100 Subject: [PATCH 05/18] Add ApplicationRecordLite that is for holding is and class of AR Add ApplicationRecordLite that is for holding is and class of AR. ActiveRecord object are to huge and we need a query to obtain them, in most cases we have a class and id, which is just enough to fil la relation. No need to go to db. --- .../manager_refresh/application_record_lite.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 app/models/manager_refresh/application_record_lite.rb diff --git a/app/models/manager_refresh/application_record_lite.rb b/app/models/manager_refresh/application_record_lite.rb new file mode 100644 index 00000000000..0ed63dbfea7 --- /dev/null +++ b/app/models/manager_refresh/application_record_lite.rb @@ -0,0 +1,12 @@ +module ManagerRefresh + class ApplicationRecordLite + attr_reader :base_class_name, :id + + # ApplicationRecord is very bloaty in memory, so this class server for storing base_class and primary key + # of the ApplicationRecord, which is just enough for filling up relationships + def initialize(base_class_name, id) + @base_class_name = base_class_name + @id = id + end + end +end From 213ddcc575c308f2ae818f53ba76f7f131a7ec2c Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Wed, 22 Feb 2017 18:39:23 +0100 Subject: [PATCH 06/18] Collect attribute_references and use them for db strategy Collect attribute_references and use them for db strategy. So we are able to load a relation fro ma db without making a sql query, wrapping it to ApplicationRecordLite. --- .../manager_refresh/inventory_collection.rb | 77 +++++++++++++------ .../manager_refresh/inventory_object.rb | 3 +- 2 files changed, 56 insertions(+), 24 deletions(-) diff --git a/app/models/manager_refresh/inventory_collection.rb b/app/models/manager_refresh/inventory_collection.rb index cc7ac5ab3ba..6310d1e2f5e 100644 --- a/app/models/manager_refresh/inventory_collection.rb +++ b/app/models/manager_refresh/inventory_collection.rb @@ -1,6 +1,6 @@ module ManagerRefresh class InventoryCollection - attr_accessor :saved, :references, :data_collection_finalized + attr_accessor :saved, :references, :attribute_references, :data_collection_finalized attr_reader :model_class, :strategy, :attributes_blacklist, :attributes_whitelist, :custom_save_block, :parent, :internal_attributes, :delete_method, :data, :data_index, :dependency_attributes, :manager_ref, @@ -37,6 +37,7 @@ def initialize(model_class: nil, manager_ref: nil, association: nil, parent: nil @attributes_whitelist = Set.new @transitive_dependency_attributes = Set.new @references = Set.new + @attribute_references = Set.new @loaded_references = Set.new @db_data_index = nil @data_collection_finalized = false @@ -182,6 +183,11 @@ def inventory_object_class end def new_inventory_object(hash) + manager_ref.each do |x| + # TODO(lsmola) with some effort, we can do this, but it's complex + raise "A lazy_find with a :key can't be a part of the manager_uuid" if inventory_object_lazy?(hash[x]) && hash[x].key + end + inventory_object_class.new(self, hash) end @@ -301,6 +307,14 @@ def foreign_type_to_association_mapping end end + def association_to_base_class_mapping + return {} unless model_class + + @association_to_base_class_mapping ||= model_class.reflect_on_all_associations.each_with_object({}) do |x, obj| + obj[x.name] = x.klass.base_class.name unless x.polymorphic? + end + end + def base_class_name return "" unless model_class @@ -324,7 +338,7 @@ def inspect def scan! data.each do |inventory_object| - scan_inventory_object(inventory_object) + scan_inventory_object!(inventory_object) end end @@ -343,12 +357,6 @@ def db_collection_for_comparison # # @param manager_uuid [String] a manager_uuid of the InventoryObject we search in the local DB def find_in_db(manager_uuid) - # TODO(lsmola) selected need to contain also :keys used in other InventoryCollections pointing to this one, once - # we get list of all keys for each InventoryCollection ,we can uncomnent - # selected = [:id] + manager_ref.map { |x| model_class.reflect_on_association(x).try(:foreign_key) || x } - # selected << :type if model_class.new.respond_to? :type - # load_from_db.select(selected).find_each do |record| - # Use the cached db_data_index only data_collection_finalized?, meaning no new reference can occur if data_collection_finalized? && db_data_index return db_data_index[manager_uuid] @@ -373,6 +381,12 @@ def populate_db_data_index! # Initialize db_data_index in nil self.db_data_index ||= {} + # TODO(lsmola) selected need to contain also :keys used in other InventoryCollections pointing to this one, once + # we get list of all keys for each InventoryCollection ,we can uncomnent + # selected = [:id] + manager_ref.map { |x| model_class.reflect_on_association(x).try(:foreign_key) || x } + # selected << :type if model_class.new.respond_to? :type + # load_from_db.select(selected).find_each do |record| + # Return the the correct relation based on strategy and selection&projection case strategy when :local_db_cache_all @@ -402,6 +416,7 @@ def db_relation(selection = nil, projection = nil) arel end rel = rel.where(selection) if rel && selection + rel = rel.select(projection) if rel && projection rel end @@ -441,43 +456,59 @@ def process_db_record!(record) else stringify_reference(custom_manager_uuid.call(record)) end - db_data_index[index] = new_inventory_object(record.attributes.symbolize_keys) + + attributes = record.attributes.symbolize_keys + attribute_references.each do |ref| + # We need to fill all references that are relations, we will use a ManagerRefresh::ApplicationRecordLite which + # can be used for filling a relation and we don't need to do any query here + # TODO(lsmola) maybe loading all, not just referenced here? Otherwise this will have issue for db_cache_all + # and find used in parser + if (foreign_key = association_to_foreign_key_mapping[ref]) + base_class_name = association_to_foreign_type_mapping[ref] || association_to_base_class_mapping[ref] + id = attributes[foreign_key.to_sym] + attributes[ref] = ManagerRefresh::ApplicationRecordLite.new(base_class_name, id) + end + end + + db_data_index[index] = new_inventory_object(attributes) db_data_index[index].id = record.id end - def scan_inventory_object(inventory_object) + def scan_inventory_object!(inventory_object) inventory_object.data.each do |key, value| if value.kind_of?(Array) - value.each { |val| scan_inventory_object_attribute(key, val) } + value.each { |val| scan_inventory_object_attribute!(key, val) } else - scan_inventory_object_attribute(key, value) + scan_inventory_object_attribute!(key, value) end end end - def scan_inventory_object_attribute(key, value) - return unless inventory_object?(value) + def scan_inventory_object_attribute!(key, value) + return if !inventory_object_lazy?(value) && !inventory_object?(value) # Storing attributes and their dependencies (dependency_attributes[key] ||= Set.new) << value.inventory_collection if value.dependency? - # Storing if attribute is a transitive dependency, so a lazy_find :key results in dependency - transitive_dependency_attributes << key if transitive_dependency?(value) - # Storing a reference in the target inventory_collection, then each IC knows about all the references and can # e.g. load all the referenced uuids from a DB value.inventory_collection.references << value.to_s + + if inventory_object_lazy?(value) + # Storing if attribute is a transitive dependency, so a lazy_find :key results in dependency + transitive_dependency_attributes << key if value.transitive_dependency? + + # If we access an attribute of the value, using a :key, we want to keep a track of that + value.inventory_collection.attribute_references << value.key if value.key + end end def inventory_object?(value) - value.kind_of?(::ManagerRefresh::InventoryObjectLazy) || value.kind_of?(::ManagerRefresh::InventoryObject) + value.kind_of?(::ManagerRefresh::InventoryObject) end - def transitive_dependency?(value) - # If the dependency is inventory_collection.lazy_find(:ems_ref, :key => :stack) - # and a :stack is a relation to another object, in the InventoryObject object, - # then this dependency is considered transitive. - (value.kind_of?(::ManagerRefresh::InventoryObjectLazy) && value.transitive_dependency?) + def inventory_object_lazy?(value) + value.kind_of?(::ManagerRefresh::InventoryObjectLazy) end def validate_inventory_collection! diff --git a/app/models/manager_refresh/inventory_object.rb b/app/models/manager_refresh/inventory_object.rb index b476cff84e0..af32e2c5104 100644 --- a/app/models/manager_refresh/inventory_object.rb +++ b/app/models/manager_refresh/inventory_object.rb @@ -146,7 +146,8 @@ def allowed?(inventory_collection_scope, key) end def loadable?(value) - value.kind_of?(::ManagerRefresh::InventoryObjectLazy) || value.kind_of?(::ManagerRefresh::InventoryObject) + value.kind_of?(::ManagerRefresh::InventoryObjectLazy) || value.kind_of?(::ManagerRefresh::InventoryObject) || + value.kind_of?(::ManagerRefresh::ApplicationRecordLite) end end end From 557cf1bfe6a31df7ea218a557fe03e781e959434 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Wed, 22 Feb 2017 18:44:32 +0100 Subject: [PATCH 07/18] Extract InitDataHelper so it's reusable in other specs Extract InitDataHelper so it's reusable in other specs --- ...ntory_collections_targeted_refresh_spec.rb | 81 +---------------- .../save_inventory/init_data_helper.rb | 88 +++++++++++++++++++ 2 files changed, 90 insertions(+), 79 deletions(-) create mode 100644 spec/models/manager_refresh/save_inventory/init_data_helper.rb diff --git a/spec/models/manager_refresh/save_inventory/graph_of_inventory_collections_targeted_refresh_spec.rb b/spec/models/manager_refresh/save_inventory/graph_of_inventory_collections_targeted_refresh_spec.rb index 703e4039879..1e7520fc8e8 100644 --- a/spec/models/manager_refresh/save_inventory/graph_of_inventory_collections_targeted_refresh_spec.rb +++ b/spec/models/manager_refresh/save_inventory/graph_of_inventory_collections_targeted_refresh_spec.rb @@ -1,9 +1,11 @@ require_relative 'spec_helper' require_relative 'spec_parsed_data' +require_relative 'init_data_helper' describe ManagerRefresh::SaveInventory do include SpecHelper include SpecParsedData + include InitDataHelper ###################################################################################################################### # Spec scenarios showing saving of the inventory with Targeted refresh strategy @@ -694,85 +696,6 @@ def all_collections %i(orchestration_stacks orchestration_stacks_resources vms miq_templates key_pairs hardwares disks) end - def initialize_all_inventory_collections - # Initialize the InventoryCollections - @data = {} - all_collections.each do |collection| - @data[collection] = ::ManagerRefresh::InventoryCollection.new(send("#{collection}_init_data")) - end - end - - def initialize_inventory_collections(only_collections) - # Initialize the InventoryCollections - @data = {} - only_collections.each do |collection| - @data[collection] = ::ManagerRefresh::InventoryCollection.new(send("#{collection}_init_data", - { - :complete => false - })) - end - - (all_collections - only_collections).each do |collection| - @data[collection] = ::ManagerRefresh::InventoryCollection.new(send("#{collection}_init_data", - { - :complete => false, - :strategy => :local_db_cache_all - })) - end - end - - def orchestration_stacks_init_data(extra_attributes = {}) - # Shadowing the default blacklist so we have an automatically solved graph cycle - init_data(cloud.orchestration_stacks(extra_attributes.merge(:attributes_blacklist => []))) - end - - def orchestration_stacks_resources_init_data(extra_attributes = {}) - # Shadowing the default blacklist so we have an automatically solved graph cycle - init_data(cloud.orchestration_stacks_resources(extra_attributes)) - end - - def vms_init_data(extra_attributes = {}) - init_data(cloud.vms(extra_attributes.merge(:attributes_blacklist => []))) - end - - def miq_templates_init_data(extra_attributes = {}) - init_data(cloud.miq_templates(extra_attributes)) - end - - def key_pairs_init_data(extra_attributes = {}) - init_data(cloud.key_pairs(extra_attributes)) - end - - def hardwares_init_data(extra_attributes = {}) - init_data(cloud.hardwares(extra_attributes)) - end - - def disks_init_data(extra_attributes = {}) - init_data(cloud.disks(extra_attributes)) - end - - def cloud - ManagerRefresh::InventoryCollectionDefault::CloudManager - end - - def init_data(extra_attributes) - init_data = { - :parent => @ems, - } - - init_data.merge!(extra_attributes) - end - - def association_attributes(model_class) - # All association attributes and foreign keys of the model - model_class.reflect_on_all_associations.map { |x| [x.name, x.foreign_key] }.flatten.compact.map(&:to_sym) - end - - def custom_association_attributes - # These are associations that are not modeled in a standard rails way, e.g. the ancestry - [:parent, :genealogy_parent, :genealogy_parent_object] - end - def initialize_inventory_collection_data # Initialize the InventoryCollections data @orchestration_stack_data_0_1 = orchestration_stack_data("0_1").merge( diff --git a/spec/models/manager_refresh/save_inventory/init_data_helper.rb b/spec/models/manager_refresh/save_inventory/init_data_helper.rb new file mode 100644 index 00000000000..f6ac2c00a92 --- /dev/null +++ b/spec/models/manager_refresh/save_inventory/init_data_helper.rb @@ -0,0 +1,88 @@ +module InitDataHelper + def initialize_all_inventory_collections + # Initialize the InventoryCollections + @data = {} + all_collections.each do |collection| + @data[collection] = ::ManagerRefresh::InventoryCollection.new(send("#{collection}_init_data")) + end + end + + def initialize_inventory_collections(only_collections) + # Initialize the InventoryCollections + @data = {} + only_collections.each do |collection| + @data[collection] = ::ManagerRefresh::InventoryCollection.new(send("#{collection}_init_data", + { + :complete => false + })) + end + + (all_collections - only_collections).each do |collection| + @data[collection] = ::ManagerRefresh::InventoryCollection.new(send("#{collection}_init_data", + { + :complete => false, + :strategy => :local_db_cache_all + })) + end + end + + def orchestration_stacks_init_data(extra_attributes = {}) + # Shadowing the default blacklist so we have an automatically solved graph cycle + init_data(cloud.orchestration_stacks(extra_attributes.merge(:attributes_blacklist => []))) + end + + def orchestration_stacks_resources_init_data(extra_attributes = {}) + # Shadowing the default blacklist so we have an automatically solved graph cycle + init_data(cloud.orchestration_stacks_resources(extra_attributes)) + end + + def vms_init_data(extra_attributes = {}) + init_data(cloud.vms(extra_attributes.merge(:attributes_blacklist => []))) + end + + def miq_templates_init_data(extra_attributes = {}) + init_data(cloud.miq_templates(extra_attributes)) + end + + def key_pairs_init_data(extra_attributes = {}) + init_data(cloud.key_pairs(extra_attributes)) + end + + def hardwares_init_data(extra_attributes = {}) + init_data(cloud.hardwares(extra_attributes)) + end + + def disks_init_data(extra_attributes = {}) + init_data(cloud.disks(extra_attributes)) + end + + def network_ports_init_data(extra_attributes = {}) + init_data(network.network_ports(extra_attributes)) + end + + def cloud + ManagerRefresh::InventoryCollectionDefault::CloudManager + end + + def network + ManagerRefresh::InventoryCollectionDefault::NetworkManager + end + + def init_data(extra_attributes) + init_data = { + :parent => @ems, + } + + init_data.merge!(extra_attributes) + end + + def association_attributes(model_class) + # All association attributes and foreign keys of the model + model_class.reflect_on_all_associations.map { |x| [x.name, x.foreign_key] }.flatten.compact.map(&:to_sym) + end + + def custom_association_attributes + # These are associations that are not modeled in a standard rails way, e.g. the ancestry + [:parent, :genealogy_parent, :genealogy_parent_object] + end +end From 0eaa8a3c9fe1151654d3084b93ad8f3a0aaea717 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Wed, 22 Feb 2017 18:45:27 +0100 Subject: [PATCH 08/18] Spec testing a db_find_strategy with :key poiting to relation Spec testing a db_find_strategy with :key poiting to relation --- .../strategies_and_references_spec.rb | 193 ++++++++++++++++++ 1 file changed, 193 insertions(+) create mode 100644 spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb diff --git a/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb b/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb new file mode 100644 index 00000000000..46b9081ea3a --- /dev/null +++ b/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb @@ -0,0 +1,193 @@ +require_relative 'spec_helper' +require_relative 'spec_parsed_data' +require_relative 'init_data_helper' + +describe ManagerRefresh::SaveInventory do + include SpecHelper + include SpecParsedData + include InitDataHelper + + ###################################################################################################################### + # Spec scenarios for different strategies and optimizations using references + ###################################################################################################################### + # + # Test all settings for ManagerRefresh::SaveInventory + [{:inventory_object_saving_strategy => nil}, + {:inventory_object_saving_strategy => :recursive}].each do |inventory_object_settings| + context "with settings #{inventory_object_settings}" do + 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(inventory_object_settings) + end + + before :each do + @image1 = FactoryGirl.create(:miq_template, image_data(1).merge(:ext_management_system => @ems)) + @image2 = FactoryGirl.create(:miq_template, image_data(2).merge(:ext_management_system => @ems)) + @image3 = FactoryGirl.create(:miq_template, image_data(3).merge(:ext_management_system => @ems)) + + @image_hardware1 = FactoryGirl.create( + :hardware, + image_hardware_data(1).merge( + :guest_os => "linux_generic_1", + :vm_or_template => @image1 + ) + ) + @image_hardware2 = FactoryGirl.create( + :hardware, + image_hardware_data(2).merge( + :guest_os => "linux_generic_2", + :vm_or_template => @image2 + ) + ) + @image_hardware3 = FactoryGirl.create( + :hardware, + image_hardware_data(3).merge( + :guest_os => "linux_generic_3", + :vm_or_template => @image3 + ) + ) + + @key_pair1 = FactoryGirl.create(:auth_key_pair_cloud, key_pair_data(1).merge(:resource => @ems)) + @key_pair12 = FactoryGirl.create(:auth_key_pair_cloud, key_pair_data(12).merge(:resource => @ems)) + @key_pair2 = FactoryGirl.create(:auth_key_pair_cloud, key_pair_data(2).merge(:resource => @ems)) + @key_pair3 = FactoryGirl.create(:auth_key_pair_cloud, key_pair_data(3).merge(:resource => @ems)) + + @vm1 = FactoryGirl.create( + :vm_cloud, + vm_data(1).merge( + :flavor => @flavor_1, + :genealogy_parent => @image1, + :key_pairs => [@key_pair1], + :location => 'host_10_10_10_1.com', + ) + ) + @vm12 = FactoryGirl.create( + :vm_cloud, + vm_data(12).merge( + :flavor => @flavor1, + :genealogy_parent => @image1, + :key_pairs => [@key_pair1, @key_pair12], + :location => 'host_10_10_10_12.com', + ) + ) + @vm2 = FactoryGirl.create( + :vm_cloud, + vm_data(2).merge( + :flavor => @flavor2, + :genealogy_parent => @image2, + :key_pairs => [@key_pair2], + :location => 'host_10_10_10_2.com', + ) + ) + @vm4 = FactoryGirl.create( + :vm_cloud, + vm_data(4).merge( + :location => 'default_value_unknown', + :ext_management_system => @ems + ) + ) + + @hardware1 = FactoryGirl.create( + :hardware, + hardware_data(1).merge( + :guest_os => @image1.hardware.guest_os, + :vm_or_template => @vm1 + ) + ) + @hardware12 = FactoryGirl.create( + :hardware, + hardware_data(12).merge( + :guest_os => @image1.hardware.guest_os, + :vm_or_template => @vm12 + ) + ) + @hardware2 = FactoryGirl.create( + :hardware, + hardware_data(2).merge( + :guest_os => @image2.hardware.guest_os, + :vm_or_template => @vm2 + ) + ) + + @network_port1 = FactoryGirl.create( + :network_port, + network_port_data(1).merge( + :device => @vm1 + ) + ) + + @network_port12 = FactoryGirl.create( + :network_port, + network_port_data(12).merge( + :device => @vm1 + ) + ) + + @network_port2 = FactoryGirl.create( + :network_port, + network_port_data(12).merge( + :device => @vm2 + ) + ) + end + + it "tests that a key pointing to relation is filled correctly when coming from db" do + vm_refs = ["vm_ems_ref_3", "vm_ems_ref_4"] + network_port_refs = ["network_port_ems_ref_1"] + + # Setup InventoryCollections + @data = {} + @data[:vms] = ::ManagerRefresh::InventoryCollection.new( + vms_init_data( + :arel => @ems.vms.where(:ems_ref => vm_refs), + :strategy => :local_db_find_references, + ) + ) + @data[:hardwares] = ::ManagerRefresh::InventoryCollection.new( + hardwares_init_data( + :arel => @ems.hardwares.joins(:vm_or_template).where(:vms => {:ems_ref => vm_refs}), + :strategy => :local_db_find_references) + ) + @data[:miq_templates] = ::ManagerRefresh::InventoryCollection.new( + miq_templates_init_data( + :strategy => :local_db_find_references) + ) + @data[:network_ports] = ::ManagerRefresh::InventoryCollection.new( + network_ports_init_data( + :parent => @ems.network_manager, + :arel => @ems.network_manager.network_ports.where(:ems_ref => network_port_refs), + :strategy => :local_db_find_missing_references) + ) + @data[:key_pairs] = ::ManagerRefresh::InventoryCollection.new( + key_pairs_init_data( + :strategy => :local_db_find_references) + ) + + @network_port_data_1 = network_port_data(1).merge( + :device => @data[:hardwares].lazy_find(vm_data(1)[:ems_ref], :key => :vm_or_template) + ) + + # Fill InventoryCollections with data + add_data_to_inventory_collection(@data[:network_ports], + @network_port_data_1) + + # Assert data before save + @network_port1.device = nil + @network_port1.save + @network_port1.reload + expect(@network_port1.device).to eq nil + + # Invoke the InventoryCollections saving + ManagerRefresh::SaveInventory.save_inventory(@ems, @data.values) + + # Asset saved data + @network_port1.reload + @vm1.reload + expect(@network_port1.device).to eq @vm1 + end + end + end +end From 0434b75cad87c01cca5f188941768f211681f89f Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Wed, 22 Feb 2017 19:37:15 +0100 Subject: [PATCH 09/18] Autofix rubocop issues Autofix rubocop issues --- .../manager_refresh/inventory_collection.rb | 9 ++-- ...ntory_collections_targeted_refresh_spec.rb | 3 +- .../save_inventory/init_data_helper.rb | 10 ++--- .../strategies_and_references_spec.rb | 44 ++++++++++--------- 4 files changed, 33 insertions(+), 33 deletions(-) diff --git a/app/models/manager_refresh/inventory_collection.rb b/app/models/manager_refresh/inventory_collection.rb index 6310d1e2f5e..127ac883351 100644 --- a/app/models/manager_refresh/inventory_collection.rb +++ b/app/models/manager_refresh/inventory_collection.rb @@ -463,11 +463,10 @@ def process_db_record!(record) # can be used for filling a relation and we don't need to do any query here # TODO(lsmola) maybe loading all, not just referenced here? Otherwise this will have issue for db_cache_all # and find used in parser - if (foreign_key = association_to_foreign_key_mapping[ref]) - base_class_name = association_to_foreign_type_mapping[ref] || association_to_base_class_mapping[ref] - id = attributes[foreign_key.to_sym] - attributes[ref] = ManagerRefresh::ApplicationRecordLite.new(base_class_name, id) - end + next unless (foreign_key = association_to_foreign_key_mapping[ref]) + base_class_name = association_to_foreign_type_mapping[ref] || association_to_base_class_mapping[ref] + id = attributes[foreign_key.to_sym] + attributes[ref] = ManagerRefresh::ApplicationRecordLite.new(base_class_name, id) end db_data_index[index] = new_inventory_object(attributes) diff --git a/spec/models/manager_refresh/save_inventory/graph_of_inventory_collections_targeted_refresh_spec.rb b/spec/models/manager_refresh/save_inventory/graph_of_inventory_collections_targeted_refresh_spec.rb index 1e7520fc8e8..9395891711e 100644 --- a/spec/models/manager_refresh/save_inventory/graph_of_inventory_collections_targeted_refresh_spec.rb +++ b/spec/models/manager_refresh/save_inventory/graph_of_inventory_collections_targeted_refresh_spec.rb @@ -320,7 +320,8 @@ hardwares_init_data( :arel => @ems.hardwares.joins(:vm_or_template).where(:vms => {:ems_ref => vm_refs}), :strategy => :local_db_find_missing_references, - :manager_ref => [:vm_or_template]) + :manager_ref => [:vm_or_template] + ) ) @data[:disks] = ::ManagerRefresh::InventoryCollection.new( disks_init_data( diff --git a/spec/models/manager_refresh/save_inventory/init_data_helper.rb b/spec/models/manager_refresh/save_inventory/init_data_helper.rb index f6ac2c00a92..e70b539ea86 100644 --- a/spec/models/manager_refresh/save_inventory/init_data_helper.rb +++ b/spec/models/manager_refresh/save_inventory/init_data_helper.rb @@ -12,17 +12,13 @@ def initialize_inventory_collections(only_collections) @data = {} only_collections.each do |collection| @data[collection] = ::ManagerRefresh::InventoryCollection.new(send("#{collection}_init_data", - { - :complete => false - })) + :complete => false)) end (all_collections - only_collections).each do |collection| @data[collection] = ::ManagerRefresh::InventoryCollection.new(send("#{collection}_init_data", - { - :complete => false, - :strategy => :local_db_cache_all - })) + :complete => false, + :strategy => :local_db_cache_all)) end end diff --git a/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb b/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb index 46b9081ea3a..4e622ebecb6 100644 --- a/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb +++ b/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb @@ -58,28 +58,28 @@ @vm1 = FactoryGirl.create( :vm_cloud, vm_data(1).merge( - :flavor => @flavor_1, - :genealogy_parent => @image1, - :key_pairs => [@key_pair1], - :location => 'host_10_10_10_1.com', + :flavor => @flavor_1, + :genealogy_parent => @image1, + :key_pairs => [@key_pair1], + :location => 'host_10_10_10_1.com', ) ) @vm12 = FactoryGirl.create( :vm_cloud, vm_data(12).merge( - :flavor => @flavor1, - :genealogy_parent => @image1, - :key_pairs => [@key_pair1, @key_pair12], - :location => 'host_10_10_10_12.com', + :flavor => @flavor1, + :genealogy_parent => @image1, + :key_pairs => [@key_pair1, @key_pair12], + :location => 'host_10_10_10_12.com', ) ) @vm2 = FactoryGirl.create( :vm_cloud, vm_data(2).merge( - :flavor => @flavor2, - :genealogy_parent => @image2, - :key_pairs => [@key_pair2], - :location => 'host_10_10_10_2.com', + :flavor => @flavor2, + :genealogy_parent => @image2, + :key_pairs => [@key_pair2], + :location => 'host_10_10_10_2.com', ) ) @vm4 = FactoryGirl.create( @@ -112,14 +112,14 @@ ) ) - @network_port1 = FactoryGirl.create( + @network_port1 = FactoryGirl.create( :network_port, network_port_data(1).merge( :device => @vm1 ) ) - @network_port12 = FactoryGirl.create( + @network_port12 = FactoryGirl.create( :network_port, network_port_data(12).merge( :device => @vm1 @@ -142,28 +142,32 @@ @data = {} @data[:vms] = ::ManagerRefresh::InventoryCollection.new( vms_init_data( - :arel => @ems.vms.where(:ems_ref => vm_refs), + :arel => @ems.vms.where(:ems_ref => vm_refs), :strategy => :local_db_find_references, ) ) @data[:hardwares] = ::ManagerRefresh::InventoryCollection.new( hardwares_init_data( - :arel => @ems.hardwares.joins(:vm_or_template).where(:vms => {:ems_ref => vm_refs}), - :strategy => :local_db_find_references) + :arel => @ems.hardwares.joins(:vm_or_template).where(:vms => {:ems_ref => vm_refs}), + :strategy => :local_db_find_references + ) ) @data[:miq_templates] = ::ManagerRefresh::InventoryCollection.new( miq_templates_init_data( - :strategy => :local_db_find_references) + :strategy => :local_db_find_references + ) ) @data[:network_ports] = ::ManagerRefresh::InventoryCollection.new( network_ports_init_data( :parent => @ems.network_manager, :arel => @ems.network_manager.network_ports.where(:ems_ref => network_port_refs), - :strategy => :local_db_find_missing_references) + :strategy => :local_db_find_missing_references + ) ) @data[:key_pairs] = ::ManagerRefresh::InventoryCollection.new( key_pairs_init_data( - :strategy => :local_db_find_references) + :strategy => :local_db_find_references + ) ) @network_port_data_1 = network_port_data(1).merge( From d9bb158421d1a1ea59ffb0a4d61f246360a3a836 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Wed, 22 Feb 2017 19:51:29 +0100 Subject: [PATCH 10/18] Fix a polymorphic relation loading Fix a polymorphic relation loading --- app/models/manager_refresh/inventory_collection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/manager_refresh/inventory_collection.rb b/app/models/manager_refresh/inventory_collection.rb index 127ac883351..cb62498c4d6 100644 --- a/app/models/manager_refresh/inventory_collection.rb +++ b/app/models/manager_refresh/inventory_collection.rb @@ -464,7 +464,7 @@ def process_db_record!(record) # TODO(lsmola) maybe loading all, not just referenced here? Otherwise this will have issue for db_cache_all # and find used in parser next unless (foreign_key = association_to_foreign_key_mapping[ref]) - base_class_name = association_to_foreign_type_mapping[ref] || association_to_base_class_mapping[ref] + base_class_name = attributes[association_to_foreign_type_mapping[ref].try(:to_sym)] || association_to_base_class_mapping[ref] id = attributes[foreign_key.to_sym] attributes[ref] = ManagerRefresh::ApplicationRecordLite.new(base_class_name, id) end From fdd94b81d33a0b6e7381c02223ae6286e4acd45f Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Thu, 23 Feb 2017 08:37:10 +0100 Subject: [PATCH 11/18] Test a polymorphic relation is obtained via :key Test a polymorphic relation is obtained via :key --- .../strategies_and_references_spec.rb | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb b/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb index 4e622ebecb6..a1d2b9761ed 100644 --- a/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb +++ b/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb @@ -128,13 +128,13 @@ @network_port2 = FactoryGirl.create( :network_port, - network_port_data(12).merge( + network_port_data(2).merge( :device => @vm2 ) ) end - it "tests that a key pointing to relation is filled correctly when coming from db" do + it "tests that a key pointing to a relation is filled correctly when coming from db" do vm_refs = ["vm_ems_ref_3", "vm_ems_ref_4"] network_port_refs = ["network_port_ems_ref_1"] @@ -192,6 +192,48 @@ @vm1.reload expect(@network_port1.device).to eq @vm1 end + + it "tests that a key pointing to a polymorphic relation is filled correctly when coming from db" do + network_port_refs = ["network_port_ems_ref_1"] + + # Setup InventoryCollections + @data = {} + @data[:network_ports] = ::ManagerRefresh::InventoryCollection.new( + network_ports_init_data( + :parent => @ems.network_manager, + :arel => @ems.network_manager.network_ports.where(:ems_ref => network_port_refs), + :strategy => :local_db_find_missing_references + ) + ) + @data[:db_network_ports] = ::ManagerRefresh::InventoryCollection.new( + network_ports_init_data( + :parent => @ems.network_manager, + :strategy => :local_db_find_references + ) + ) + + @network_port_data_1 = network_port_data(1).merge( + :device => @data[:db_network_ports].lazy_find(network_port_data(12)[:ems_ref], :key => :device) + ) + + # Fill InventoryCollections with data + add_data_to_inventory_collection(@data[:network_ports], + @network_port_data_1) + + # Assert data before save + @network_port1.device = nil + @network_port1.save + @network_port1.reload + expect(@network_port1.device).to eq nil + + # Invoke the InventoryCollections saving + ManagerRefresh::SaveInventory.save_inventory(@ems, @data.values) + + # Asset saved data + @network_port1.reload + @vm1.reload + expect(@network_port1.device).to eq @vm1 + end end end end From 9cb0f7390b421edb1c6490a3b7f37b56a515c34b Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Thu, 23 Feb 2017 10:20:59 +0100 Subject: [PATCH 12/18] Clean up a spec to only use a needed ICs Clean up a spec to only use a needed ICs --- .../strategies_and_references_spec.rb | 22 +++---------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb b/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb index a1d2b9761ed..bcc5d4a3ee9 100644 --- a/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb +++ b/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb @@ -140,23 +140,6 @@ # Setup InventoryCollections @data = {} - @data[:vms] = ::ManagerRefresh::InventoryCollection.new( - vms_init_data( - :arel => @ems.vms.where(:ems_ref => vm_refs), - :strategy => :local_db_find_references, - ) - ) - @data[:hardwares] = ::ManagerRefresh::InventoryCollection.new( - hardwares_init_data( - :arel => @ems.hardwares.joins(:vm_or_template).where(:vms => {:ems_ref => vm_refs}), - :strategy => :local_db_find_references - ) - ) - @data[:miq_templates] = ::ManagerRefresh::InventoryCollection.new( - miq_templates_init_data( - :strategy => :local_db_find_references - ) - ) @data[:network_ports] = ::ManagerRefresh::InventoryCollection.new( network_ports_init_data( :parent => @ems.network_manager, @@ -164,8 +147,9 @@ :strategy => :local_db_find_missing_references ) ) - @data[:key_pairs] = ::ManagerRefresh::InventoryCollection.new( - key_pairs_init_data( + @data[:hardwares] = ::ManagerRefresh::InventoryCollection.new( + hardwares_init_data( + :arel => @ems.hardwares.joins(:vm_or_template).where(:vms => {:ems_ref => vm_refs}), :strategy => :local_db_find_references ) ) From bb3dbd83f0ec4797f70ad5349406f07963a456b6 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Thu, 23 Feb 2017 14:37:13 +0100 Subject: [PATCH 13/18] Look for foreign_key only in belongs_to relations Look for foreign_key only in belongs_to relations, we would get a confusing trying to assign to another relation. --- app/models/manager_refresh/inventory_collection.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/models/manager_refresh/inventory_collection.rb b/app/models/manager_refresh/inventory_collection.rb index cb62498c4d6..2c576ada961 100644 --- a/app/models/manager_refresh/inventory_collection.rb +++ b/app/models/manager_refresh/inventory_collection.rb @@ -275,10 +275,14 @@ def clone :dependency_attributes => dependency_attributes.clone) end + def belongs_to_associations + model_class.reflect_on_all_associations.select { |x| x.kind_of? ActiveRecord::Reflection::BelongsToReflection } + end + def association_to_foreign_key_mapping return {} unless model_class - @association_to_foreign_key_mapping ||= model_class.reflect_on_all_associations.each_with_object({}) do |x, obj| + @association_to_foreign_key_mapping ||= belongs_to_associations.each_with_object({}) do |x, obj| obj[x.name] = x.foreign_key end end @@ -286,7 +290,7 @@ def association_to_foreign_key_mapping def foreign_key_to_association_mapping return {} unless model_class - @foreign_key_to_association_mapping ||= model_class.reflect_on_all_associations.each_with_object({}) do |x, obj| + @foreign_key_to_association_mapping ||= belongs_to_associations.each_with_object({}) do |x, obj| obj[x.foreign_key] = x.name end end From ebc99212465c4c0bca5a11effebd3062c44813a3 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Thu, 23 Feb 2017 14:38:21 +0100 Subject: [PATCH 14/18] Check for allowed strategies Check for allowed strategies and raise exception with a list of allowed --- app/models/manager_refresh/inventory_collection.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/models/manager_refresh/inventory_collection.rb b/app/models/manager_refresh/inventory_collection.rb index 2c576ada961..61b68265005 100644 --- a/app/models/manager_refresh/inventory_collection.rb +++ b/app/models/manager_refresh/inventory_collection.rb @@ -57,6 +57,8 @@ def to_hash end def process_strategy(strategy_name) + return unless strategy_name + case strategy_name when :local_db_cache_all self.data_collection_finalized = true @@ -64,6 +66,9 @@ def process_strategy(strategy_name) when :local_db_find_references self.saved = true when :local_db_find_missing_references + else + raise "Unknown InventoryCollection strategy: :#{strategy_name}, allowed strategies are :local_db_cache_all, "\ + ":local_db_find_references and :local_db_find_missing_references." end strategy_name end From cf20b74fce5fdf5d537f75a7101aadf3607d826a Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Thu, 23 Feb 2017 11:58:06 +0100 Subject: [PATCH 15/18] A complex interconnection spec A complex interconnection spec --- .../strategies_and_references_spec.rb | 121 +++++++++++++++++- 1 file changed, 119 insertions(+), 2 deletions(-) diff --git a/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb b/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb index bcc5d4a3ee9..e37cb00c94d 100644 --- a/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb +++ b/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb @@ -132,6 +132,13 @@ :device => @vm2 ) ) + + @network_port4 = FactoryGirl.create( + :network_port, + network_port_data(4).merge( + :device => @vm4 + ) + ) end it "tests that a key pointing to a relation is filled correctly when coming from db" do @@ -154,6 +161,7 @@ ) ) + # Parse data for InventoryCollections @network_port_data_1 = network_port_data(1).merge( :device => @data[:hardwares].lazy_find(vm_data(1)[:ems_ref], :key => :vm_or_template) ) @@ -171,7 +179,7 @@ # Invoke the InventoryCollections saving ManagerRefresh::SaveInventory.save_inventory(@ems, @data.values) - # Asset saved data + # Assert saved data @network_port1.reload @vm1.reload expect(@network_port1.device).to eq @vm1 @@ -196,6 +204,7 @@ ) ) + # Parse data for InventoryCollections @network_port_data_1 = network_port_data(1).merge( :device => @data[:db_network_ports].lazy_find(network_port_data(12)[:ems_ref], :key => :device) ) @@ -213,11 +222,119 @@ # Invoke the InventoryCollections saving ManagerRefresh::SaveInventory.save_inventory(@ems, @data.values) - # Asset saved data + # Assert saved data @network_port1.reload @vm1.reload expect(@network_port1.device).to eq @vm1 end + + it "saves records correctly with complex interconnection" do + vm_refs = ["vm_ems_ref_3", "vm_ems_ref_4"] + network_port_refs = ["network_port_ems_ref_1", "network_port_ems_ref_12"] + + # Setup InventoryCollections + @data = {} + @data[:miq_templates] = ::ManagerRefresh::InventoryCollection.new( + miq_templates_init_data( + :strategy => :local_db_find_references + ) + ) + @data[:key_pairs] = ::ManagerRefresh::InventoryCollection.new( + key_pairs_init_data( + :strategy => :local_db_find_references + ) + ) + @data[:db_network_ports] = ::ManagerRefresh::InventoryCollection.new( + network_ports_init_data( + :parent => @ems.network_manager, + :strategy => :local_db_find_references + ) + ) + @data[:vms] = ::ManagerRefresh::InventoryCollection.new( + vms_init_data( + :arel => @ems.vms.where(:ems_ref => vm_refs), + :strategy => :local_db_find_missing_references, + ) + ) + @data[:hardwares] = ::ManagerRefresh::InventoryCollection.new( + hardwares_init_data( + :arel => @ems.hardwares.joins(:vm_or_template).where(:vms => {:ems_ref => vm_refs}), + :strategy => :local_db_find_missing_references + ) + ) + @data[:network_ports] = ::ManagerRefresh::InventoryCollection.new( + network_ports_init_data( + :parent => @ems.network_manager, + :arel => @ems.network_manager.network_ports.where(:ems_ref => network_port_refs), + :strategy => :local_db_find_missing_references + ) + ) + + # Parse data for InventoryCollections + @network_port_data_1 = network_port_data(1).merge( + :name => @data[:vms].lazy_find(vm_data(3)[:ems_ref], :key => :name), + :device => @data[:vms].lazy_find(vm_data(3)[:ems_ref]) + ) + @network_port_data_12 = network_port_data(12).merge( + :name => @data[:vms].lazy_find(vm_data(4)[:ems_ref], :key => :name, :default => "default_name"), + :device => @data[:db_network_ports].lazy_find(network_port_data(2)[:ems_ref], :key => :device) + ) + @network_port_data_3 = network_port_data(3).merge( + :name => @data[:vms].lazy_find(vm_data(1)[:ems_ref], :key => :name, :default => "default_name"), + :device => @data[:hardwares].lazy_find(vm_data(1)[:ems_ref], :key => :vm_or_template) + ) + @vm_data_3 = vm_data(3).merge( + :genealogy_parent => @data[:miq_templates].lazy_find(image_data(2)[:ems_ref]), + :key_pairs => [@data[:key_pairs].lazy_find(key_pair_data(2)[:name])], + :ext_management_system => @ems + ) + @hardware_data_3 = hardware_data(3).merge( + :guest_os => @data[:hardwares].lazy_find(image_data(2)[:ems_ref], :key => :guest_os), + :vm_or_template => @data[:vms].lazy_find(vm_data(3)[:ems_ref]) + ) + + # Fill InventoryCollections with data + add_data_to_inventory_collection(@data[:network_ports], + @network_port_data_1, + @network_port_data_12, + @network_port_data_3) + add_data_to_inventory_collection(@data[:vms], + @vm_data_3) + add_data_to_inventory_collection(@data[:hardwares], + @hardware_data_3) + # Assert data before save + expect(@network_port1.device).to eq @vm1 + expect(@network_port1.name).to eq "network_port_name_1" + + expect(@network_port12.device).to eq @vm1 + expect(@network_port12.name).to eq "network_port_name_12" + + expect(@vm4.ext_management_system).to eq @ems + + # Invoke the InventoryCollections saving + ManagerRefresh::SaveInventory.save_inventory(@ems, @data.values) + + # Assert saved data + @vm3 = Vm.find_by(:ems_ref => vm_data(3)[:ems_ref]) + @vm4 = Vm.find_by(:ems_ref => vm_data(4)[:ems_ref]) + @network_port3 = NetworkPort.find_by(:ems_ref => network_port_data(3)[:ems_ref]) + @network_port1.reload + @network_port12.reload + @vm4.reload + # @image2.reload will not refresh STI class + @image2 = MiqTemplate.find(@image2.id) + + expect(@network_port1.device).to eq @vm3 + expect(@network_port1.name).to eq "vm_name_3" + expect(@network_port12.device).to eq @vm2 + # Vm4 name was not found, because @vm4 got disconnected and no longer can be found in ems.vms + expect(@network_port12.name).to eq "default_name" + expect(@network_port3.device).to eq @vm1 + expect(@network_port3.name).to eq "vm_name_1" + expect(@vm3.genealogy_parent).to eq @image2 + # Check Vm4 was disconnected + expect(@vm4.ext_management_system).to be_nil + end end end end From d26d696938a90f0b96d126e0b83eadb34884d6e6 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Thu, 23 Feb 2017 14:49:20 +0100 Subject: [PATCH 16/18] Enhance specs and run them for both DB strategies Enhance specs and run them for both DB strategies --- .../strategies_and_references_spec.rb | 667 +++++++++--------- 1 file changed, 348 insertions(+), 319 deletions(-) diff --git a/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb b/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb index e37cb00c94d..7c74bf22376 100644 --- a/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb +++ b/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb @@ -15,325 +15,354 @@ [{:inventory_object_saving_strategy => nil}, {:inventory_object_saving_strategy => :recursive}].each do |inventory_object_settings| context "with settings #{inventory_object_settings}" do - 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(inventory_object_settings) - end - - before :each do - @image1 = FactoryGirl.create(:miq_template, image_data(1).merge(:ext_management_system => @ems)) - @image2 = FactoryGirl.create(:miq_template, image_data(2).merge(:ext_management_system => @ems)) - @image3 = FactoryGirl.create(:miq_template, image_data(3).merge(:ext_management_system => @ems)) - - @image_hardware1 = FactoryGirl.create( - :hardware, - image_hardware_data(1).merge( - :guest_os => "linux_generic_1", - :vm_or_template => @image1 - ) - ) - @image_hardware2 = FactoryGirl.create( - :hardware, - image_hardware_data(2).merge( - :guest_os => "linux_generic_2", - :vm_or_template => @image2 - ) - ) - @image_hardware3 = FactoryGirl.create( - :hardware, - image_hardware_data(3).merge( - :guest_os => "linux_generic_3", - :vm_or_template => @image3 - ) - ) - - @key_pair1 = FactoryGirl.create(:auth_key_pair_cloud, key_pair_data(1).merge(:resource => @ems)) - @key_pair12 = FactoryGirl.create(:auth_key_pair_cloud, key_pair_data(12).merge(:resource => @ems)) - @key_pair2 = FactoryGirl.create(:auth_key_pair_cloud, key_pair_data(2).merge(:resource => @ems)) - @key_pair3 = FactoryGirl.create(:auth_key_pair_cloud, key_pair_data(3).merge(:resource => @ems)) - - @vm1 = FactoryGirl.create( - :vm_cloud, - vm_data(1).merge( - :flavor => @flavor_1, - :genealogy_parent => @image1, - :key_pairs => [@key_pair1], - :location => 'host_10_10_10_1.com', - ) - ) - @vm12 = FactoryGirl.create( - :vm_cloud, - vm_data(12).merge( - :flavor => @flavor1, - :genealogy_parent => @image1, - :key_pairs => [@key_pair1, @key_pair12], - :location => 'host_10_10_10_12.com', - ) - ) - @vm2 = FactoryGirl.create( - :vm_cloud, - vm_data(2).merge( - :flavor => @flavor2, - :genealogy_parent => @image2, - :key_pairs => [@key_pair2], - :location => 'host_10_10_10_2.com', - ) - ) - @vm4 = FactoryGirl.create( - :vm_cloud, - vm_data(4).merge( - :location => 'default_value_unknown', - :ext_management_system => @ems - ) - ) - - @hardware1 = FactoryGirl.create( - :hardware, - hardware_data(1).merge( - :guest_os => @image1.hardware.guest_os, - :vm_or_template => @vm1 - ) - ) - @hardware12 = FactoryGirl.create( - :hardware, - hardware_data(12).merge( - :guest_os => @image1.hardware.guest_os, - :vm_or_template => @vm12 - ) - ) - @hardware2 = FactoryGirl.create( - :hardware, - hardware_data(2).merge( - :guest_os => @image2.hardware.guest_os, - :vm_or_template => @vm2 - ) - ) - - @network_port1 = FactoryGirl.create( - :network_port, - network_port_data(1).merge( - :device => @vm1 - ) - ) - - @network_port12 = FactoryGirl.create( - :network_port, - network_port_data(12).merge( - :device => @vm1 - ) - ) - - @network_port2 = FactoryGirl.create( - :network_port, - network_port_data(2).merge( - :device => @vm2 - ) - ) - - @network_port4 = FactoryGirl.create( - :network_port, - network_port_data(4).merge( - :device => @vm4 - ) - ) - end - - it "tests that a key pointing to a relation is filled correctly when coming from db" do - vm_refs = ["vm_ems_ref_3", "vm_ems_ref_4"] - network_port_refs = ["network_port_ems_ref_1"] - - # Setup InventoryCollections - @data = {} - @data[:network_ports] = ::ManagerRefresh::InventoryCollection.new( - network_ports_init_data( - :parent => @ems.network_manager, - :arel => @ems.network_manager.network_ports.where(:ems_ref => network_port_refs), - :strategy => :local_db_find_missing_references - ) - ) - @data[:hardwares] = ::ManagerRefresh::InventoryCollection.new( - hardwares_init_data( - :arel => @ems.hardwares.joins(:vm_or_template).where(:vms => {:ems_ref => vm_refs}), - :strategy => :local_db_find_references - ) - ) - - # Parse data for InventoryCollections - @network_port_data_1 = network_port_data(1).merge( - :device => @data[:hardwares].lazy_find(vm_data(1)[:ems_ref], :key => :vm_or_template) - ) - - # Fill InventoryCollections with data - add_data_to_inventory_collection(@data[:network_ports], - @network_port_data_1) - - # Assert data before save - @network_port1.device = nil - @network_port1.save - @network_port1.reload - expect(@network_port1.device).to eq nil - - # Invoke the InventoryCollections saving - ManagerRefresh::SaveInventory.save_inventory(@ems, @data.values) - - # Assert saved data - @network_port1.reload - @vm1.reload - expect(@network_port1.device).to eq @vm1 - end - - it "tests that a key pointing to a polymorphic relation is filled correctly when coming from db" do - network_port_refs = ["network_port_ems_ref_1"] - - # Setup InventoryCollections - @data = {} - @data[:network_ports] = ::ManagerRefresh::InventoryCollection.new( - network_ports_init_data( - :parent => @ems.network_manager, - :arel => @ems.network_manager.network_ports.where(:ems_ref => network_port_refs), - :strategy => :local_db_find_missing_references - ) - ) - @data[:db_network_ports] = ::ManagerRefresh::InventoryCollection.new( - network_ports_init_data( - :parent => @ems.network_manager, - :strategy => :local_db_find_references - ) - ) - - # Parse data for InventoryCollections - @network_port_data_1 = network_port_data(1).merge( - :device => @data[:db_network_ports].lazy_find(network_port_data(12)[:ems_ref], :key => :device) - ) - - # Fill InventoryCollections with data - add_data_to_inventory_collection(@data[:network_ports], - @network_port_data_1) - - # Assert data before save - @network_port1.device = nil - @network_port1.save - @network_port1.reload - expect(@network_port1.device).to eq nil - - # Invoke the InventoryCollections saving - ManagerRefresh::SaveInventory.save_inventory(@ems, @data.values) - - # Assert saved data - @network_port1.reload - @vm1.reload - expect(@network_port1.device).to eq @vm1 - end - - it "saves records correctly with complex interconnection" do - vm_refs = ["vm_ems_ref_3", "vm_ems_ref_4"] - network_port_refs = ["network_port_ems_ref_1", "network_port_ems_ref_12"] - - # Setup InventoryCollections - @data = {} - @data[:miq_templates] = ::ManagerRefresh::InventoryCollection.new( - miq_templates_init_data( - :strategy => :local_db_find_references - ) - ) - @data[:key_pairs] = ::ManagerRefresh::InventoryCollection.new( - key_pairs_init_data( - :strategy => :local_db_find_references - ) - ) - @data[:db_network_ports] = ::ManagerRefresh::InventoryCollection.new( - network_ports_init_data( - :parent => @ems.network_manager, - :strategy => :local_db_find_references - ) - ) - @data[:vms] = ::ManagerRefresh::InventoryCollection.new( - vms_init_data( - :arel => @ems.vms.where(:ems_ref => vm_refs), - :strategy => :local_db_find_missing_references, - ) - ) - @data[:hardwares] = ::ManagerRefresh::InventoryCollection.new( - hardwares_init_data( - :arel => @ems.hardwares.joins(:vm_or_template).where(:vms => {:ems_ref => vm_refs}), - :strategy => :local_db_find_missing_references - ) - ) - @data[:network_ports] = ::ManagerRefresh::InventoryCollection.new( - network_ports_init_data( - :parent => @ems.network_manager, - :arel => @ems.network_manager.network_ports.where(:ems_ref => network_port_refs), - :strategy => :local_db_find_missing_references - ) - ) - - # Parse data for InventoryCollections - @network_port_data_1 = network_port_data(1).merge( - :name => @data[:vms].lazy_find(vm_data(3)[:ems_ref], :key => :name), - :device => @data[:vms].lazy_find(vm_data(3)[:ems_ref]) - ) - @network_port_data_12 = network_port_data(12).merge( - :name => @data[:vms].lazy_find(vm_data(4)[:ems_ref], :key => :name, :default => "default_name"), - :device => @data[:db_network_ports].lazy_find(network_port_data(2)[:ems_ref], :key => :device) - ) - @network_port_data_3 = network_port_data(3).merge( - :name => @data[:vms].lazy_find(vm_data(1)[:ems_ref], :key => :name, :default => "default_name"), - :device => @data[:hardwares].lazy_find(vm_data(1)[:ems_ref], :key => :vm_or_template) - ) - @vm_data_3 = vm_data(3).merge( - :genealogy_parent => @data[:miq_templates].lazy_find(image_data(2)[:ems_ref]), - :key_pairs => [@data[:key_pairs].lazy_find(key_pair_data(2)[:name])], - :ext_management_system => @ems - ) - @hardware_data_3 = hardware_data(3).merge( - :guest_os => @data[:hardwares].lazy_find(image_data(2)[:ems_ref], :key => :guest_os), - :vm_or_template => @data[:vms].lazy_find(vm_data(3)[:ems_ref]) - ) - - # Fill InventoryCollections with data - add_data_to_inventory_collection(@data[:network_ports], - @network_port_data_1, - @network_port_data_12, - @network_port_data_3) - add_data_to_inventory_collection(@data[:vms], - @vm_data_3) - add_data_to_inventory_collection(@data[:hardwares], - @hardware_data_3) - # Assert data before save - expect(@network_port1.device).to eq @vm1 - expect(@network_port1.name).to eq "network_port_name_1" - - expect(@network_port12.device).to eq @vm1 - expect(@network_port12.name).to eq "network_port_name_12" - - expect(@vm4.ext_management_system).to eq @ems - - # Invoke the InventoryCollections saving - ManagerRefresh::SaveInventory.save_inventory(@ems, @data.values) - - # Assert saved data - @vm3 = Vm.find_by(:ems_ref => vm_data(3)[:ems_ref]) - @vm4 = Vm.find_by(:ems_ref => vm_data(4)[:ems_ref]) - @network_port3 = NetworkPort.find_by(:ems_ref => network_port_data(3)[:ems_ref]) - @network_port1.reload - @network_port12.reload - @vm4.reload - # @image2.reload will not refresh STI class - @image2 = MiqTemplate.find(@image2.id) - - expect(@network_port1.device).to eq @vm3 - expect(@network_port1.name).to eq "vm_name_3" - expect(@network_port12.device).to eq @vm2 - # Vm4 name was not found, because @vm4 got disconnected and no longer can be found in ems.vms - expect(@network_port12.name).to eq "default_name" - expect(@network_port3.device).to eq @vm1 - expect(@network_port3.name).to eq "vm_name_1" - expect(@vm3.genealogy_parent).to eq @image2 - # Check Vm4 was disconnected - expect(@vm4.ext_management_system).to be_nil + [:local_db_find_references, :local_db_cache_all].each do |db_strategy| + context "with db strategy #{db_strategy}" do + + 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(inventory_object_settings) + end + + before :each do + @image1 = FactoryGirl.create(:miq_template, image_data(1).merge(:ext_management_system => @ems)) + @image2 = FactoryGirl.create(:miq_template, image_data(2).merge(:ext_management_system => @ems)) + @image3 = FactoryGirl.create(:miq_template, image_data(3).merge(:ext_management_system => @ems)) + + @image_hardware1 = FactoryGirl.create( + :hardware, + image_hardware_data(1).merge( + :guest_os => "linux_generic_1", + :vm_or_template => @image1 + ) + ) + @image_hardware2 = FactoryGirl.create( + :hardware, + image_hardware_data(2).merge( + :guest_os => "linux_generic_2", + :vm_or_template => @image2 + ) + ) + @image_hardware3 = FactoryGirl.create( + :hardware, + image_hardware_data(3).merge( + :guest_os => "linux_generic_3", + :vm_or_template => @image3 + ) + ) + + @key_pair1 = FactoryGirl.create(:auth_key_pair_cloud, key_pair_data(1).merge(:resource => @ems)) + @key_pair12 = FactoryGirl.create(:auth_key_pair_cloud, key_pair_data(12).merge(:resource => @ems)) + @key_pair2 = FactoryGirl.create(:auth_key_pair_cloud, key_pair_data(2).merge(:resource => @ems)) + @key_pair3 = FactoryGirl.create(:auth_key_pair_cloud, key_pair_data(3).merge(:resource => @ems)) + + @vm1 = FactoryGirl.create( + :vm_cloud, + vm_data(1).merge( + :flavor => @flavor_1, + :genealogy_parent => @image1, + :key_pairs => [@key_pair1], + :location => 'host_10_10_10_1.com', + ) + ) + @vm12 = FactoryGirl.create( + :vm_cloud, + vm_data(12).merge( + :flavor => @flavor1, + :genealogy_parent => @image1, + :key_pairs => [@key_pair1, @key_pair12], + :location => 'host_10_10_10_12.com', + ) + ) + @vm2 = FactoryGirl.create( + :vm_cloud, + vm_data(2).merge( + :flavor => @flavor2, + :genealogy_parent => @image2, + :key_pairs => [@key_pair2], + :location => 'host_10_10_10_2.com', + ) + ) + @vm4 = FactoryGirl.create( + :vm_cloud, + vm_data(4).merge( + :location => 'default_value_unknown', + :ext_management_system => @ems + ) + ) + + @hardware1 = FactoryGirl.create( + :hardware, + hardware_data(1).merge( + :guest_os => @image1.hardware.guest_os, + :vm_or_template => @vm1 + ) + ) + @hardware12 = FactoryGirl.create( + :hardware, + hardware_data(12).merge( + :guest_os => @image1.hardware.guest_os, + :vm_or_template => @vm12 + ) + ) + @hardware2 = FactoryGirl.create( + :hardware, + hardware_data(2).merge( + :guest_os => @image2.hardware.guest_os, + :vm_or_template => @vm2 + ) + ) + + @network_port1 = FactoryGirl.create( + :network_port, + network_port_data(1).merge( + :device => @vm1 + ) + ) + + @network_port12 = FactoryGirl.create( + :network_port, + network_port_data(12).merge( + :device => @vm1 + ) + ) + + @network_port2 = FactoryGirl.create( + :network_port, + network_port_data(2).merge( + :device => @vm2 + ) + ) + + @network_port4 = FactoryGirl.create( + :network_port, + network_port_data(4).merge( + :device => @vm4 + ) + ) + end + + it "tests that a key pointing to a relation is filled correctly when coming from db" do + vm_refs = ["vm_ems_ref_3", "vm_ems_ref_4"] + network_port_refs = ["network_port_ems_ref_1"] + + # Setup InventoryCollections + @data = {} + @data[:network_ports] = ::ManagerRefresh::InventoryCollection.new( + network_ports_init_data( + :parent => @ems.network_manager, + :arel => @ems.network_manager.network_ports.where(:ems_ref => network_port_refs), + :strategy => :local_db_find_missing_references + ) + ) + @data[:hardwares] = ::ManagerRefresh::InventoryCollection.new( + hardwares_init_data( + :arel => @ems.hardwares.joins(:vm_or_template).where(:vms => {:ems_ref => vm_refs}), + :strategy => db_strategy + ) + ) + + # Parse data for InventoryCollections + @network_port_data_1 = network_port_data(1).merge( + :device => @data[:hardwares].lazy_find(vm_data(1)[:ems_ref], :key => :vm_or_template) + ) + + # Fill InventoryCollections with data + add_data_to_inventory_collection(@data[:network_ports], + @network_port_data_1) + + # Assert data before save + @network_port1.device = nil + @network_port1.save + @network_port1.reload + expect(@network_port1.device).to eq nil + + # Invoke the InventoryCollections saving + ManagerRefresh::SaveInventory.save_inventory(@ems, @data.values) + + # Assert saved data + @network_port1.reload + @vm1.reload + expect(@network_port1.device).to eq @vm1 + end + + it "tests that a key pointing to a polymorphic relation is filled correctly when coming from db" do + network_port_refs = ["network_port_ems_ref_1"] + + # Setup InventoryCollections + @data = {} + @data[:network_ports] = ::ManagerRefresh::InventoryCollection.new( + network_ports_init_data( + :parent => @ems.network_manager, + :arel => @ems.network_manager.network_ports.where(:ems_ref => network_port_refs), + :strategy => :local_db_find_missing_references + ) + ) + @data[:db_network_ports] = ::ManagerRefresh::InventoryCollection.new( + network_ports_init_data( + :parent => @ems.network_manager, + :strategy => db_strategy + ) + ) + + # Parse data for InventoryCollections + @network_port_data_1 = network_port_data(1).merge( + :device => @data[:db_network_ports].lazy_find(network_port_data(12)[:ems_ref], :key => :device) + ) + + # Fill InventoryCollections with data + add_data_to_inventory_collection(@data[:network_ports], + @network_port_data_1) + + # Assert data before save + @network_port1.device = nil + @network_port1.save + @network_port1.reload + expect(@network_port1.device).to eq nil + + # Invoke the InventoryCollections saving + ManagerRefresh::SaveInventory.save_inventory(@ems, @data.values) + + # Assert saved data + @network_port1.reload + @vm1.reload + expect(@network_port1.device).to eq @vm1 + end + + it "saves records correctly with complex interconnection" do + vm_refs = ["vm_ems_ref_3", "vm_ems_ref_4"] + network_port_refs = ["network_port_ems_ref_1", "network_port_ems_ref_12"] + + # Setup InventoryCollections + @data = {} + @data[:miq_templates] = ::ManagerRefresh::InventoryCollection.new( + miq_templates_init_data( + :strategy => db_strategy + ) + ) + @data[:key_pairs] = ::ManagerRefresh::InventoryCollection.new( + key_pairs_init_data( + :strategy => db_strategy + ) + ) + @data[:db_network_ports] = ::ManagerRefresh::InventoryCollection.new( + network_ports_init_data( + :parent => @ems.network_manager, + :strategy => db_strategy + ) + ) + @data[:db_vms] = ::ManagerRefresh::InventoryCollection.new( + vms_init_data( + :strategy => db_strategy + ) + ) + @data[:vms] = ::ManagerRefresh::InventoryCollection.new( + vms_init_data( + :arel => @ems.vms.where(:ems_ref => vm_refs), + :strategy => :local_db_find_missing_references, + ) + ) + @data[:hardwares] = ::ManagerRefresh::InventoryCollection.new( + hardwares_init_data( + :arel => @ems.hardwares.joins(:vm_or_template).where(:vms => {:ems_ref => vm_refs}), + :strategy => :local_db_find_missing_references + ) + ) + @data[:network_ports] = ::ManagerRefresh::InventoryCollection.new( + network_ports_init_data( + :parent => @ems.network_manager, + :arel => @ems.network_manager.network_ports.where(:ems_ref => network_port_refs), + :strategy => :local_db_find_missing_references + ) + ) + + # Parse data for InventoryCollections + @network_port_data_1 = network_port_data(1).merge( + :name => @data[:vms].lazy_find(vm_data(3)[:ems_ref], :key => :name), + :device => @data[:vms].lazy_find(vm_data(3)[:ems_ref]) + ) + @network_port_data_12 = network_port_data(12).merge( + :name => @data[:vms].lazy_find(vm_data(4)[:ems_ref], :key => :name, :default => "default_name"), + :device => @data[:db_network_ports].lazy_find(network_port_data(2)[:ems_ref], :key => :device) + ) + @network_port_data_3 = network_port_data(3).merge( + :name => @data[:vms].lazy_find(vm_data(1)[:ems_ref], :key => :name, :default => "default_name"), + :device => @data[:hardwares].lazy_find(vm_data(1)[:ems_ref], :key => :vm_or_template) + ) + @vm_data_3 = vm_data(3).merge( + :genealogy_parent => @data[:miq_templates].lazy_find(image_data(2)[:ems_ref]), + :key_pairs => [ + @data[:key_pairs].lazy_find(key_pair_data(2)[:name]), + @data[:key_pairs].lazy_find(key_pair_data(3)[:name]) + ], + :ext_management_system => @ems + ) + @vm_data_31 = vm_data(31).merge( + :genealogy_parent => @data[:miq_templates].lazy_find(image_data(2)[:ems_ref]), + :key_pairs => @data[:db_vms].lazy_find(vm_data(1)[:ems_ref], :key => :key_pairs, :default => []), + :ext_management_system => @ems + ) + @hardware_data_3 = hardware_data(3).merge( + :guest_os => @data[:hardwares].lazy_find(image_data(2)[:ems_ref], :key => :guest_os), + :vm_or_template => @data[:vms].lazy_find(vm_data(3)[:ems_ref]) + ) + + # Fill InventoryCollections with data + add_data_to_inventory_collection(@data[:network_ports], + @network_port_data_1, + @network_port_data_12, + @network_port_data_3) + add_data_to_inventory_collection(@data[:vms], + @vm_data_3, + @vm_data_31) + add_data_to_inventory_collection(@data[:hardwares], + @hardware_data_3) + # Assert data before save + expect(@network_port1.device).to eq @vm1 + expect(@network_port1.name).to eq "network_port_name_1" + + expect(@network_port12.device).to eq @vm1 + expect(@network_port12.name).to eq "network_port_name_12" + + expect(@vm4.ext_management_system).to eq @ems + + # Invoke the InventoryCollections saving + ManagerRefresh::SaveInventory.save_inventory(@ems, @data.values) + + #### Assert saved data #### + @vm3 = Vm.find_by(:ems_ref => vm_data(3)[:ems_ref]) + @vm31 = Vm.find_by(:ems_ref => vm_data(31)[:ems_ref]) + @vm4 = Vm.find_by(:ems_ref => vm_data(4)[:ems_ref]) + @network_port3 = NetworkPort.find_by(:ems_ref => network_port_data(3)[:ems_ref]) + @network_port1.reload + @network_port12.reload + @vm4.reload + # @image2.reload will not refresh STI class, we should probably extend the factory with the right class + @image2 = MiqTemplate.find(@image2.id) + + expect(@network_port1.device).to eq @vm3 + expect(@network_port1.name).to eq "vm_name_3" + expect(@network_port12.device).to eq @vm2 + # Vm4 name was not found, because @vm4 got disconnected and no longer can be found in ems.vms + expect(@network_port12.name).to eq "default_name" + expect(@network_port3.device).to eq @vm1 + expect(@network_port3.name).to eq "vm_name_1" + expect(@vm3.genealogy_parent).to eq @image2 + expect(@vm3.key_pairs).to match_array [@key_pair2, @key_pair3] + expect(@vm3.hardware.guest_os).to eq "linux_generic_2" + expect(@vm31.genealogy_parent).to eq @image2 + # We don't support :key pointing to a has_many, so it default to [] + expect(@vm31.key_pairs).to match_array [] + expect(@vm31.hardware).to be_nil + # Check Vm4 was disconnected + expect(@vm4.ext_management_system).to be_nil + end + end end end end From 3d976dafc3864d9e7f2d1161d36dd8c39e8acc2b Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Thu, 23 Feb 2017 16:26:48 +0100 Subject: [PATCH 17/18] Autofix rubocop issues Autofix rubocop issues --- .../strategies_and_references_spec.rb | 38 +++++++++---------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb b/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb index 7c74bf22376..cf27f8b5bd1 100644 --- a/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb +++ b/spec/models/manager_refresh/save_inventory/strategies_and_references_spec.rb @@ -17,13 +17,11 @@ context "with settings #{inventory_object_settings}" do [:local_db_find_references, :local_db_cache_all].each do |db_strategy| context "with db strategy #{db_strategy}" do - before :each do @zone = FactoryGirl.create(:zone) @ems = FactoryGirl.create(:ems_cloud, :zone => @zone, - :network_manager => FactoryGirl.create(:ems_network, :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(inventory_object_settings) @@ -61,7 +59,7 @@ @key_pair2 = FactoryGirl.create(:auth_key_pair_cloud, key_pair_data(2).merge(:resource => @ems)) @key_pair3 = FactoryGirl.create(:auth_key_pair_cloud, key_pair_data(3).merge(:resource => @ems)) - @vm1 = FactoryGirl.create( + @vm1 = FactoryGirl.create( :vm_cloud, vm_data(1).merge( :flavor => @flavor_1, @@ -79,7 +77,7 @@ :location => 'host_10_10_10_12.com', ) ) - @vm2 = FactoryGirl.create( + @vm2 = FactoryGirl.create( :vm_cloud, vm_data(2).merge( :flavor => @flavor2, @@ -88,7 +86,7 @@ :location => 'host_10_10_10_2.com', ) ) - @vm4 = FactoryGirl.create( + @vm4 = FactoryGirl.create( :vm_cloud, vm_data(4).merge( :location => 'default_value_unknown', @@ -96,7 +94,7 @@ ) ) - @hardware1 = FactoryGirl.create( + @hardware1 = FactoryGirl.create( :hardware, hardware_data(1).merge( :guest_os => @image1.hardware.guest_os, @@ -110,7 +108,7 @@ :vm_or_template => @vm12 ) ) - @hardware2 = FactoryGirl.create( + @hardware2 = FactoryGirl.create( :hardware, hardware_data(2).merge( :guest_os => @image2.hardware.guest_os, @@ -160,7 +158,7 @@ :strategy => :local_db_find_missing_references ) ) - @data[:hardwares] = ::ManagerRefresh::InventoryCollection.new( + @data[:hardwares] = ::ManagerRefresh::InventoryCollection.new( hardwares_init_data( :arel => @ems.hardwares.joins(:vm_or_template).where(:vms => {:ems_ref => vm_refs}), :strategy => db_strategy @@ -168,7 +166,7 @@ ) # Parse data for InventoryCollections - @network_port_data_1 = network_port_data(1).merge( + @network_port_data_1 = network_port_data(1).merge( :device => @data[:hardwares].lazy_find(vm_data(1)[:ems_ref], :key => :vm_or_template) ) @@ -211,7 +209,7 @@ ) # Parse data for InventoryCollections - @network_port_data_1 = network_port_data(1).merge( + @network_port_data_1 = network_port_data(1).merge( :device => @data[:db_network_ports].lazy_find(network_port_data(12)[:ems_ref], :key => :device) ) @@ -245,7 +243,7 @@ :strategy => db_strategy ) ) - @data[:key_pairs] = ::ManagerRefresh::InventoryCollection.new( + @data[:key_pairs] = ::ManagerRefresh::InventoryCollection.new( key_pairs_init_data( :strategy => db_strategy ) @@ -256,24 +254,24 @@ :strategy => db_strategy ) ) - @data[:db_vms] = ::ManagerRefresh::InventoryCollection.new( + @data[:db_vms] = ::ManagerRefresh::InventoryCollection.new( vms_init_data( :strategy => db_strategy ) ) - @data[:vms] = ::ManagerRefresh::InventoryCollection.new( + @data[:vms] = ::ManagerRefresh::InventoryCollection.new( vms_init_data( :arel => @ems.vms.where(:ems_ref => vm_refs), :strategy => :local_db_find_missing_references, ) ) - @data[:hardwares] = ::ManagerRefresh::InventoryCollection.new( + @data[:hardwares] = ::ManagerRefresh::InventoryCollection.new( hardwares_init_data( :arel => @ems.hardwares.joins(:vm_or_template).where(:vms => {:ems_ref => vm_refs}), :strategy => :local_db_find_missing_references ) ) - @data[:network_ports] = ::ManagerRefresh::InventoryCollection.new( + @data[:network_ports] = ::ManagerRefresh::InventoryCollection.new( network_ports_init_data( :parent => @ems.network_manager, :arel => @ems.network_manager.network_ports.where(:ems_ref => network_port_refs), @@ -282,15 +280,15 @@ ) # Parse data for InventoryCollections - @network_port_data_1 = network_port_data(1).merge( + @network_port_data_1 = network_port_data(1).merge( :name => @data[:vms].lazy_find(vm_data(3)[:ems_ref], :key => :name), :device => @data[:vms].lazy_find(vm_data(3)[:ems_ref]) ) - @network_port_data_12 = network_port_data(12).merge( + @network_port_data_12 = network_port_data(12).merge( :name => @data[:vms].lazy_find(vm_data(4)[:ems_ref], :key => :name, :default => "default_name"), :device => @data[:db_network_ports].lazy_find(network_port_data(2)[:ems_ref], :key => :device) ) - @network_port_data_3 = network_port_data(3).merge( + @network_port_data_3 = network_port_data(3).merge( :name => @data[:vms].lazy_find(vm_data(1)[:ems_ref], :key => :name, :default => "default_name"), :device => @data[:hardwares].lazy_find(vm_data(1)[:ems_ref], :key => :vm_or_template) ) @@ -307,7 +305,7 @@ :key_pairs => @data[:db_vms].lazy_find(vm_data(1)[:ems_ref], :key => :key_pairs, :default => []), :ext_management_system => @ems ) - @hardware_data_3 = hardware_data(3).merge( + @hardware_data_3 = hardware_data(3).merge( :guest_os => @data[:hardwares].lazy_find(image_data(2)[:ems_ref], :key => :guest_os), :vm_or_template => @data[:vms].lazy_find(vm_data(3)[:ems_ref]) ) From 19e7dcc92596307098467702b623c64ea0eb9ae3 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Thu, 23 Feb 2017 16:50:30 +0100 Subject: [PATCH 18/18] Rename ApplicationRecordLite to ApplicationRecordReference Rename ApplicationRecordLite to ApplicationRecordReference, as it shows the purpose much better. --- ...ication_record_lite.rb => application_record_reference.rb} | 2 +- app/models/manager_refresh/inventory_collection.rb | 4 ++-- app/models/manager_refresh/inventory_object.rb | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) rename app/models/manager_refresh/{application_record_lite.rb => application_record_reference.rb} (91%) diff --git a/app/models/manager_refresh/application_record_lite.rb b/app/models/manager_refresh/application_record_reference.rb similarity index 91% rename from app/models/manager_refresh/application_record_lite.rb rename to app/models/manager_refresh/application_record_reference.rb index 0ed63dbfea7..e3cf4fe7ff3 100644 --- a/app/models/manager_refresh/application_record_lite.rb +++ b/app/models/manager_refresh/application_record_reference.rb @@ -1,5 +1,5 @@ module ManagerRefresh - class ApplicationRecordLite + class ApplicationRecordReference attr_reader :base_class_name, :id # ApplicationRecord is very bloaty in memory, so this class server for storing base_class and primary key diff --git a/app/models/manager_refresh/inventory_collection.rb b/app/models/manager_refresh/inventory_collection.rb index 61b68265005..a9d00a1d114 100644 --- a/app/models/manager_refresh/inventory_collection.rb +++ b/app/models/manager_refresh/inventory_collection.rb @@ -468,14 +468,14 @@ def process_db_record!(record) attributes = record.attributes.symbolize_keys attribute_references.each do |ref| - # We need to fill all references that are relations, we will use a ManagerRefresh::ApplicationRecordLite which + # We need to fill all references that are relations, we will use a ManagerRefresh::ApplicationRecordReference which # can be used for filling a relation and we don't need to do any query here # TODO(lsmola) maybe loading all, not just referenced here? Otherwise this will have issue for db_cache_all # and find used in parser next unless (foreign_key = association_to_foreign_key_mapping[ref]) base_class_name = attributes[association_to_foreign_type_mapping[ref].try(:to_sym)] || association_to_base_class_mapping[ref] id = attributes[foreign_key.to_sym] - attributes[ref] = ManagerRefresh::ApplicationRecordLite.new(base_class_name, id) + attributes[ref] = ManagerRefresh::ApplicationRecordReference.new(base_class_name, id) end db_data_index[index] = new_inventory_object(attributes) diff --git a/app/models/manager_refresh/inventory_object.rb b/app/models/manager_refresh/inventory_object.rb index af32e2c5104..a2c4a68093f 100644 --- a/app/models/manager_refresh/inventory_object.rb +++ b/app/models/manager_refresh/inventory_object.rb @@ -147,7 +147,7 @@ def allowed?(inventory_collection_scope, key) def loadable?(value) value.kind_of?(::ManagerRefresh::InventoryObjectLazy) || value.kind_of?(::ManagerRefresh::InventoryObject) || - value.kind_of?(::ManagerRefresh::ApplicationRecordLite) + value.kind_of?(::ManagerRefresh::ApplicationRecordReference) end end end