From fa88308ea45d6dd5af9019a2855c4d7a768dd122 Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Thu, 26 Apr 2018 14:40:31 +0200
Subject: [PATCH 01/18] Inventory Collection Builder

New Interface for definition/creating InventoryCollection from persister.
Accessible via add_collection() method
---
 .../manager_refresh/inventory/persister.rb    |  40 +++
 .../manager_refresh/inventory_collection.rb   |   3 +-
 .../inventory_collection/builder.rb           | 228 ++++++++++++++++++
 .../builder/cloud_manager.rb                  | 176 ++++++++++++++
 .../builder/infra_manager.rb                  | 212 ++++++++++++++++
 .../builder/network_manager.rb                | 208 ++++++++++++++++
 .../inventory_collection/builder/shared.rb    |  90 +++++++
 .../builder/storage_manager.rb                |   9 +
 .../manager_refresh/inventory_object.rb       |  15 +-
 9 files changed, 976 insertions(+), 5 deletions(-)
 create mode 100644 app/models/manager_refresh/inventory_collection/builder.rb
 create mode 100644 app/models/manager_refresh/inventory_collection/builder/cloud_manager.rb
 create mode 100644 app/models/manager_refresh/inventory_collection/builder/infra_manager.rb
 create mode 100644 app/models/manager_refresh/inventory_collection/builder/network_manager.rb
 create mode 100644 app/models/manager_refresh/inventory_collection/builder/shared.rb
 create mode 100644 app/models/manager_refresh/inventory_collection/builder/storage_manager.rb

diff --git a/app/models/manager_refresh/inventory/persister.rb b/app/models/manager_refresh/inventory/persister.rb
index ae8aaae2eb9..299a601bcf4 100644
--- a/app/models/manager_refresh/inventory/persister.rb
+++ b/app/models/manager_refresh/inventory/persister.rb
@@ -123,6 +123,46 @@ def shared_options
     {}
   end
 
+  # Interface for creating InventoryCollection under @collections
+  #
+  # @param builder_class [ManagerRefresh::InventoryCollection::Builder]
+  # @param collection_name [Symbol] used as InventoryCollection:association
+  # @param extra_properties [Hash] props from InventoryCollection.initialize list
+  #         - overwrites all previous settings
+  # @param settings [Hash] builder settings
+  #         - @see ManagerRefresh::InventoryCollection::Builder.default_options
+  #         - @see make_builder_settings()
+  #
+  # @example
+  #   add_collection(ManagerRefresh::InventoryCollection::Builder::CloudManager, :vms) do |builder|
+  #     builder.add_properties(
+  #       :strategy => :local_db_cache_all,
+  #     )
+  #   )
+  def add_collection(builder_class, collection_name, extra_properties = {}, settings = {}, &block)
+    builder = builder_class.prepare_data(collection_name,
+                                         self.class,
+                                         make_builder_settings(settings),
+                                         &block)
+
+    builder.add_properties(extra_properties) if extra_properties.present?
+    builder.add_properties({:parent => manager}, :missing) if manager.present?
+
+    builder.transform_builder_params(self)
+
+    collections[collection_name] = builder.to_inventory_collection
+  end
+
+  # @param extra_settings [Hash]
+  def make_builder_settings(extra_settings = {})
+    opts = ::ManagerRefresh::InventoryCollection::Builder.default_options
+
+    opts[:adv_settings] = options[:inventory_collections].try(:to_hash) || {}
+    opts[:shared_properties] = shared_options
+
+    opts.merge(extra_settings)
+  end
+
   # Adds 1 ManagerRefresh::InventoryCollection under a target.collections using :association key as index
   #
   # @param options [Hash] Hash used for ManagerRefresh::InventoryCollection initialize
diff --git a/app/models/manager_refresh/inventory_collection.rb b/app/models/manager_refresh/inventory_collection.rb
index 433122d9fee..561336d971b 100644
--- a/app/models/manager_refresh/inventory_collection.rb
+++ b/app/models/manager_refresh/inventory_collection.rb
@@ -44,6 +44,8 @@ module ManagerRefresh
   #   puts @ems.vms.collect(&:ems_ref) # => ["vm2", "vm3"]
   #
   class InventoryCollection
+    require_nested :Builder
+
     # @return [Boolean] A true value marks that we collected all the data of the InventoryCollection,
     #         meaning we also collected all the references.
     attr_accessor :data_collection_finalized
@@ -445,7 +447,6 @@ def initialize(model_class: nil, manager_ref: nil, association: nil, parent: nil
       @attributes_whitelist             = Set.new
       @transitive_dependency_attributes = Set.new
       @dependees                        = Set.new
-
       @data_storage = ::ManagerRefresh::InventoryCollection::DataStorage.new(self, secondary_refs)
       @references_storage = ::ManagerRefresh::InventoryCollection::ReferencesStorage.new(index_proxy)
       @targeted_scope = ::ManagerRefresh::InventoryCollection::ReferencesStorage.new(index_proxy).merge!(manager_uuids)
diff --git a/app/models/manager_refresh/inventory_collection/builder.rb b/app/models/manager_refresh/inventory_collection/builder.rb
new file mode 100644
index 00000000000..0cce59bc23e
--- /dev/null
+++ b/app/models/manager_refresh/inventory_collection/builder.rb
@@ -0,0 +1,228 @@
+module ManagerRefresh
+  class InventoryCollection
+    class Builder
+      class MissingModelClassError < StandardError
+      end
+
+      require_nested :CloudManager
+      require_nested :NetworkManager
+      require_nested :StorageManager
+      require_nested :InfraManager
+
+      include ::ManagerRefresh::InventoryCollection::Builder::Shared
+
+      # Default options for builder
+      #   :auto_object_attributes
+      #     - auto creates inventory_object_attributes from target model_class setters
+      #     - attributes used in InventoryObject.add_attributes
+      #   :auto_model_class
+      #     - tries to set model_class from persister class
+      #     - @see method auto_model_class
+      #   :without_model_class
+      #     - if false and no model_class derived or specified, throws exception
+      #   :adv_settings
+      #     - values from Advanced settings (doesn't overwrite values specified in code)
+      #     - see persister options() method
+      def self.default_options
+        {
+          :auto_object_attributes => true,
+          :auto_model_class       => true,
+          :without_model_class    => false,
+          :adv_settings           => true
+        }
+      end
+
+      # Entry point
+      # Creates builder and builds data for inventory collection
+      # @param name [Symbol] InventoryCollection.association value
+      #        (optional) method with this name also used for concrete inventory collection specific properties
+      # @param persister_class [Class] used for "guessing" model_class
+      # @param options [Hash]
+      def self.prepare_data(name, persister_class, options = default_options, &block)
+        builder = new(name, persister_class, options)
+        builder.construct_data(&block)
+        builder
+      end
+
+      # @see prepare_data()
+      def initialize(name, persister_class, options = self.class.default_options)
+        @name = name
+        @persister_class = persister_class
+
+        @properties = {}
+        @inventory_object_attributes = []
+        @builder_params = {}
+        @dependency_attributes = {}
+
+        @options = options
+        @adv_settings = options[:adv_settings] # Configuration/Advanced settings in GUI
+        @shared_properties = options[:shared_properties] # From persister
+      end
+
+      # Builds data for InventoryCollection
+      # Calls method @name (if exists) with specific properties
+      # Yields for overwriting provider-specific properties
+      def construct_data
+        add_properties(:association => @name)
+        add_properties(:model_class => auto_model_class) if @options[:auto_model_class]
+
+        add_properties(@adv_settings, :missing)
+        add_properties(@shared_properties, :missing)
+
+        send(@name.to_sym) if respond_to?(@name.to_sym)
+
+        yield(self) if block_given?
+
+        add_inventory_attributes(auto_object_attributes) if @options[:auto_object_attributes]
+      end
+
+      # Creates InventoryCollection
+      def to_inventory_collection
+        if @properties[:model_class].nil? && !@options[:without_model_class]
+          raise MissingModelClassError
+        end
+
+        ::ManagerRefresh::InventoryCollection.new(to_hash)
+      end
+
+      #
+      # Missing method *add_some_property(value)* converted to:
+      # add_properties(:some_property => value)
+      #
+      def method_missing(method_name, *arguments, &block)
+        if method_name.to_s.starts_with?('add_')
+          add_properties(
+            method_name.to_s.gsub('add_', '').to_sym => arguments[0]
+          )
+        else
+          super
+        end
+      end
+
+      def respond_to_missing?(method_name, _include_private = false)
+        method_name.to_s.starts_with?('add_')
+      end
+
+      # Merges @properties
+      # @see ManagerRefresh::InventoryCollection.initialize for list of properties
+      #
+      # @param props [Hash]
+      # @param mode [Symbol] :overwrite | :missing
+      def add_properties(props = {}, mode = :overwrite)
+        @properties = merge_hashes(@properties, props, mode)
+      end
+
+      # Adds inventory object attributes (part of @properties)
+      def add_inventory_attributes(array)
+        @inventory_object_attributes += (array || [])
+      end
+
+      # Removes specified inventory object attributes
+      def remove_inventory_attributes(array)
+        @inventory_object_attributes -= (array || [])
+      end
+
+      # Clears all inventory object attributes
+      def clear_attributes!
+        @inventory_object_attributes = []
+      end
+
+      # Adds key/values to builder params (part of @properties)
+      def add_builder_params(params = {}, mode = :overwrite)
+        @builder_params = merge_hashes(@builder_params, params, mode)
+      end
+
+      # Evaluates lambda blocks in @builder_params
+      def transform_builder_params(persister)
+        if @builder_params
+          @builder_params = @builder_params.transform_values do |value|
+            if value.respond_to?(:call)
+              value.call(persister)
+            else
+              value
+            end
+          end
+        end
+      end
+
+      # Adds key/values to dependency_attributes (part of @properties)
+      def add_dependency_attributes(attrs = {}, mode = :overwrite)
+        @dependency_attributes = merge_hashes(@dependency_attributes, attrs, mode)
+      end
+
+      # Returns whole InventoryCollection properties
+      def to_hash
+        @properties.merge(
+          :inventory_object_attributes => @inventory_object_attributes,
+          :builder_params              => @builder_params,
+          :dependency_attributes       => @dependency_attributes
+        )
+      end
+
+      protected
+
+      # Extends source hash with
+      # - a) all keys from dest (overwrite mode)
+      # - b) missing keys (missing mode)
+      #
+      # @param mode [Symbol] :overwrite | :missing
+      def merge_hashes(source, dest, mode)
+        return source if source.nil? || dest.nil?
+
+        if mode == :overwrite
+          source.merge(dest)
+        else
+          dest.merge(source)
+        end
+      end
+
+      # Derives model_class from persister class and @name
+      # 1) searches for class in provider
+      # 2) if not found, searches class in core
+      # Can be disabled by options :auto_model_class => false
+      #
+      # @example derives model_class from amazon
+      #
+      #   @persister_class = ManageIQ::Providers::Amazon::Inventory::Persister::CloudManager
+      #   @name = :vms
+      #
+      #   returns - <provider_module>::<manager_module>::<@name.classify>
+      #   returns - ::ManageIQ::Providers::Amazon::CloudManager::Vm
+      #
+      # @example derives model_class from @name only
+      #
+      #   @persister_class = ManagerRefresh::Inventory::Persister
+      #   @name = :vms
+      #
+      #   returns ::Vm
+      #
+      # @return [Class | nil] when class doesn't exist, returns nil
+      def auto_model_class
+        # a) Provider specific class
+        provider_module = ManageIQ::Providers::Inflector.provider_module(@persister_class).name
+        manager_module = self.class.name.split('::').last
+
+        class_name = "#{provider_module}::#{manager_module}::#{@name.to_s.classify}"
+        model_class = class_name.safe_constantize
+        if model_class
+          model_class
+        else
+          # b) general class
+          "::#{@name.to_s.classify}".safe_constantize
+        end
+      end
+
+      # Inventory object attributes are derived from setters
+      #
+      # Can be disabled by options :auto_object_attributes => false
+      #   - attributes can be manually set via method add_inventory_attributes()
+      def auto_object_attributes
+        return if @properties[:model_class].nil?
+
+        (@properties[:model_class].new.methods - ApplicationRecord.methods).grep(/^[\w]+?\=$/).collect do |setter|
+          setter.to_s[0..setter.length - 2].to_sym
+        end
+      end
+    end
+  end
+end
diff --git a/app/models/manager_refresh/inventory_collection/builder/cloud_manager.rb b/app/models/manager_refresh/inventory_collection/builder/cloud_manager.rb
new file mode 100644
index 00000000000..fe024acbe97
--- /dev/null
+++ b/app/models/manager_refresh/inventory_collection/builder/cloud_manager.rb
@@ -0,0 +1,176 @@
+module ManagerRefresh
+  class InventoryCollection
+    class Builder
+      class CloudManager < ::ManagerRefresh::InventoryCollection::Builder
+        def availability_zones
+          shared_builder_params
+        end
+
+        def flavors
+          shared_builder_params
+        end
+
+        def key_pairs
+          add_properties(
+            :model_class => ::ManageIQ::Providers::CloudManager::AuthKeyPair,
+            :manager_ref => %i(name)
+          )
+          add_builder_params(
+            :resource_id   => ->(persister) { persister.manager.id },
+            :resource_type => ->(persister) { persister.manager.class.base_class }
+          )
+        end
+
+        def vm_and_template_labels
+          # TODO(lsmola) make a generic CustomAttribute IC and move it to base class
+          add_properties(
+            :model_class                  => ::CustomAttribute,
+            :manager_ref                  => %i(resource name),
+            :parent_inventory_collections => %i(vms miq_templates)
+          )
+        end
+
+        def orchestration_stacks
+          add_properties(
+            :model_class          => ::ManageIQ::Providers::CloudManager::OrchestrationStack,
+            :attributes_blacklist => %i(parent),
+          )
+
+          shared_builder_params
+        end
+
+        def orchestration_stacks_resources
+          add_properties(
+            :model_class                  => ::OrchestrationStackResource,
+            :parent_inventory_collections => %i(orchestration_stacks)
+          )
+        end
+
+        def orchestration_stacks_outputs
+          add_properties(
+            :model_class                  => ::OrchestrationStackOutput,
+            :parent_inventory_collections => %i(orchestration_stacks)
+          )
+        end
+
+        def orchestration_stacks_parameters
+          add_properties(
+            :model_class                  => ::OrchestrationStackParameter,
+            :parent_inventory_collections => %i(orchestration_stacks)
+          )
+        end
+
+        def orchestration_templates
+          # TODO(lsmola) do refactoring, we shouldn't need this custom saving block\
+          orchestration_templates_save_block = lambda do |_ems, inventory_collection|
+            hashes = inventory_collection.data.map(&:attributes)
+
+            templates = inventory_collection.model_class.find_or_create_by_contents(hashes)
+            inventory_collection.data.zip(templates).each do |inventory_object, template|
+              inventory_object.id = template.id
+            end
+          end
+
+          add_properties(
+            :custom_save_block => orchestration_templates_save_block
+          )
+        end
+
+        def orchestration_stack_ancestry
+          orchestration_stack_ancestry_save_block = lambda do |_ems, inventory_collection|
+            stacks_inventory_collection = inventory_collection.dependency_attributes[:orchestration_stacks].try(:first)
+
+            return if stacks_inventory_collection.blank?
+
+            stacks_parents = stacks_inventory_collection.data.each_with_object({}) do |x, obj|
+              parent_id = x.data[:parent].try(:load).try(:id)
+              obj[x.id] = parent_id if parent_id
+            end
+
+            model_class = stacks_inventory_collection.model_class
+
+            stacks_parents_indexed = model_class
+                                     .select(%i(id ancestry))
+                                     .where(:id => stacks_parents.values).find_each.index_by(&:id)
+
+            ActiveRecord::Base.transaction do
+              model_class.select(%i(id ancestry))
+                         .where(:id => stacks_parents.keys).find_each do |stack|
+                parent = stacks_parents_indexed[stacks_parents[stack.id]]
+                stack.update_attribute(:parent, parent)
+              end
+            end
+          end
+
+          add_properties(
+            :custom_save_block => orchestration_stack_ancestry_save_block
+          )
+        end
+
+        def vm_and_miq_template_ancestry
+          vm_and_miq_template_ancestry_save_block = lambda do |_ems, inventory_collection|
+            vms_inventory_collection = inventory_collection.dependency_attributes[:vms].try(:first)
+            miq_templates_inventory_collection = inventory_collection.dependency_attributes[:miq_templates].try(:first)
+
+            return if vms_inventory_collection.blank? || miq_templates_inventory_collection.blank?
+
+            # Fetch IDs of all vms and genealogy_parents, only if genealogy_parent is present
+            vms_genealogy_parents = vms_inventory_collection.data.each_with_object({}) do |x, obj|
+              unless x.data[:genealogy_parent].nil?
+                genealogy_parent_id = x.data[:genealogy_parent].load.try(:id)
+                obj[x.id] = genealogy_parent_id if genealogy_parent_id
+              end
+            end
+
+            miq_template_genealogy_parents = miq_templates_inventory_collection.data.each_with_object({}) do |x, obj|
+              unless x.data[:genealogy_parent].nil?
+                genealogy_parent_id = x.data[:genealogy_parent].load.try(:id)
+                obj[x.id] = genealogy_parent_id if genealogy_parent_id
+              end
+            end
+
+            ActiveRecord::Base.transaction do
+              # associate parent templates to child instances
+              parent_miq_templates = miq_templates_inventory_collection.model_class
+                                                                       .select([:id])
+                                                                       .where(:id => vms_genealogy_parents.values).find_each.index_by(&:id)
+              vms_inventory_collection.model_class
+                                      .select([:id])
+                                      .where(:id => vms_genealogy_parents.keys).find_each do |vm|
+                parent = parent_miq_templates[vms_genealogy_parents[vm.id]]
+                vm.with_relationship_type('genealogy') { vm.parent = parent }
+              end
+            end
+
+            ActiveRecord::Base.transaction do
+              # associate parent instances to child templates
+              parent_vms = vms_inventory_collection.model_class
+                                                   .select([:id])
+                                                   .where(:id => miq_template_genealogy_parents.values).find_each.index_by(&:id)
+              miq_templates_inventory_collection.model_class
+                                                .select([:id])
+                                                .where(:id => miq_template_genealogy_parents.keys).find_each do |miq_template|
+                parent = parent_vms[miq_template_genealogy_parents[miq_template.id]]
+                miq_template.with_relationship_type('genealogy') { miq_template.parent = parent }
+              end
+            end
+          end
+
+          add_properties(
+            :custom_save_block => vm_and_miq_template_ancestry_save_block
+          )
+        end
+      end
+
+      private
+
+      def shared_builder_params
+        add_builder_params(:ems_id => default_ems_id)
+      end
+
+      def default_ems_id
+        ->(persister) { persister.manager.id }
+      end
+    end
+  end
+end
diff --git a/app/models/manager_refresh/inventory_collection/builder/infra_manager.rb b/app/models/manager_refresh/inventory_collection/builder/infra_manager.rb
new file mode 100644
index 00000000000..6ad63450a64
--- /dev/null
+++ b/app/models/manager_refresh/inventory_collection/builder/infra_manager.rb
@@ -0,0 +1,212 @@
+module ManagerRefresh
+  class InventoryCollection
+    class Builder
+      class InfraManager < ::ManagerRefresh::InventoryCollection::Builder
+        def networks
+          add_properties(
+            :manager_ref => %i(hardware ipaddress ipv6address)
+          )
+        end
+
+        def host_networks
+          add_properties(
+            :model_class                  => ::Network,
+            :manager_ref                  => %i(hardware ipaddress),
+            :parent_inventory_collections => %i(hosts)
+          )
+        end
+
+        def guest_devices
+          add_properties(
+            :manager_ref => %i(hardware uid_ems),
+          )
+        end
+
+        def host_hardwares
+          add_properties(
+            :model_class                  => ::Hardware,
+            :manager_ref                  => %i(host),
+            :parent_inventory_collections => %i(hosts)
+          )
+        end
+
+        def snapshots
+          add_properties(
+            :manager_ref => %i(uid)
+          )
+        end
+
+        def operating_systems
+          add_properties(
+            :manager_ref => %i(vm_or_template)
+          )
+        end
+
+        def host_operating_systems
+          add_properties(
+            :model_class                  => ::OperatingSystem,
+            :manager_ref                  => %i(host),
+            :parent_inventory_collections => %i(hosts),
+          )
+        end
+
+        def custom_attributes
+          add_properties(
+            :manager_ref => %i(name)
+          )
+        end
+
+        def ems_folders
+          add_properties(
+            :manager_ref          => %i(uid_ems),
+            :attributes_blacklist => %i(ems_children),
+          )
+          shared_builder_params
+        end
+
+        def datacenters
+          shared_builder_params
+        end
+
+        def resource_pools
+          add_properties(
+            :manager_ref          => %i(uid_ems),
+            :attributes_blacklist => %i(ems_children),
+          )
+          shared_builder_params
+        end
+
+        def ems_clusters
+          add_properties(
+            :attributes_blacklist => %i(ems_children datacenter_id),
+          )
+          shared_builder_params
+        end
+
+        def storages
+          add_properties(
+            :manager_ref => %i(location),
+            :complete    => false,
+            :arel        => Storage,
+          )
+        end
+
+        def hosts
+          shared_builder_params
+
+          add_custom_reconnect_block(
+            lambda do |inventory_collection, inventory_objects_index, attributes_index|
+              relation = inventory_collection.model_class.where(:ems_id => nil)
+
+              return if relation.count <= 0
+
+              inventory_objects_index.each_slice(100) do |batch|
+                relation.where(inventory_collection.manager_ref.first => batch.map(&:first)).each do |record|
+                  index = inventory_collection.object_index_with_keys(inventory_collection.manager_ref_to_cols, record)
+
+                  # We need to delete the record from the inventory_objects_index and attributes_index, otherwise it
+                  # would be sent for create.
+                  inventory_object = inventory_objects_index.delete(index)
+                  hash             = attributes_index.delete(index)
+
+                  record.assign_attributes(hash.except(:id, :type))
+                  if !inventory_collection.check_changed? || record.changed?
+                    record.save!
+                    inventory_collection.store_updated_records(record)
+                  end
+
+                  inventory_object.id = record.id
+                end
+              end
+            end
+          )
+        end
+
+        def vms
+          super
+
+          add_custom_reconnect_block(
+            lambda do |inventory_collection, inventory_objects_index, attributes_index|
+              relation = inventory_collection.model_class.where(:ems_id => nil)
+
+              return if relation.count <= 0
+
+              inventory_objects_index.each_slice(100) do |batch|
+                relation.where(inventory_collection.manager_ref.first => batch.map(&:first)).each do |record|
+                  index = inventory_collection.object_index_with_keys(inventory_collection.manager_ref_to_cols, record)
+
+                  # We need to delete the record from the inventory_objects_index and attributes_index, otherwise it
+                  # would be sent for create.
+                  inventory_object = inventory_objects_index.delete(index)
+                  hash             = attributes_index.delete(index)
+
+                  record.assign_attributes(hash.except(:id, :type))
+                  if !inventory_collection.check_changed? || record.changed?
+                    record.save!
+                    inventory_collection.store_updated_records(record)
+                  end
+
+                  inventory_object.id = record.id
+                end
+              end
+            end
+          )
+        end
+
+        def host_storages
+          add_properties(
+            :manager_ref                  => %i(host storage),
+            :parent_inventory_collections => %i(hosts)
+          )
+        end
+
+        def host_switches
+          add_properties(
+            :manager_ref                  => %i(host switch),
+            :parent_inventory_collections => %i(hosts)
+          )
+        end
+
+        def switches
+          add_properties(
+            :manager_ref => %i(uid_ems)
+          )
+        end
+
+        def lans
+          add_properties(
+            :manager_ref => %i(uid_ems),
+          )
+        end
+
+        def snapshot_parent
+          snapshot_parent_save_block = lambda do |_ems, inventory_collection|
+            snapshot_collection = inventory_collection.dependency_attributes[:snapshots].try(:first)
+
+            snapshot_collection.each do |snapshot|
+              ActiveRecord::Base.transaction do
+                child = Snapshot.find(snapshot.id)
+                parent = Snapshot.find_by(:uid_ems => snapshot.parent_uid)
+                child.update_attribute(:parent_id, parent.try(:id))
+              end
+            end
+          end
+
+          add_properties(
+            :custom_save_block => snapshot_parent_save_block
+          )
+        end
+
+        private
+
+        def shared_builder_params
+          add_builder_params(:ems_id => default_ems_id)
+        end
+
+        def default_ems_id
+          ->(persister) { persister.manager.id }
+        end
+      end
+    end
+  end
+end
diff --git a/app/models/manager_refresh/inventory_collection/builder/network_manager.rb b/app/models/manager_refresh/inventory_collection/builder/network_manager.rb
new file mode 100644
index 00000000000..07849056078
--- /dev/null
+++ b/app/models/manager_refresh/inventory_collection/builder/network_manager.rb
@@ -0,0 +1,208 @@
+module ManagerRefresh
+  class InventoryCollection
+    class Builder
+      class NetworkManager < ::ManagerRefresh::InventoryCollection::Builder
+        def cloud_subnet_network_ports
+          add_properties(
+            :model_class                  => ::CloudSubnetNetworkPort,
+            :manager_ref                  => %i(address cloud_subnet network_port),
+            :parent_inventory_collections => %i(vms network_ports load_balancers)
+          )
+
+          add_targeted_arel(
+            lambda do |inventory_collection|
+              manager_uuids = inventory_collection.parent_inventory_collections.flat_map { |c| c.manager_uuids.to_a }
+              inventory_collection.parent.cloud_subnet_network_ports.references(:network_ports).where(
+                :network_ports => {:ems_ref => manager_uuids}
+              )
+            end
+          )
+        end
+
+        def network_ports
+          add_properties(
+            :use_ar_object  => true,
+            # TODO(lsmola) can't do batch strategy for network_ports because of security_groups relation
+            :saver_strategy => :default
+          )
+
+          shared_builder_params
+        end
+
+        def network_groups
+          shared_builder_params
+        end
+
+        def network_routers
+          shared_builder_params
+        end
+
+        def floating_ips
+          shared_builder_params
+        end
+
+        def cloud_subnets
+          shared_builder_params
+        end
+
+        def cloud_networks
+          shared_builder_params
+        end
+
+        def security_groups
+          shared_builder_params
+        end
+
+        def firewall_rules
+          add_properties(
+            :manager_ref                  => %i(resource source_security_group direction host_protocol port end_port source_ip_range),
+            :parent_inventory_collections => %i(security_groups)
+          )
+        end
+
+        def load_balancers
+          shared_builder_params
+        end
+
+        def load_balancer_pools
+          add_properties(
+            :parent_inventory_collections => %i(load_balancers)
+          )
+
+          add_targeted_arel(
+            lambda do |inventory_collection|
+              manager_uuids = inventory_collection.parent_inventory_collections.flat_map { |c| c.manager_uuids.to_a }
+              inventory_collection.parent.load_balancer_pools
+                .joins(:load_balancers)
+                .where(:load_balancers => {:ems_ref => manager_uuids})
+                .distinct
+            end
+          )
+
+          shared_builder_params
+        end
+
+        def load_balancer_pool_members
+          add_properties(
+            :parent_inventory_collections => %i(load_balancers)
+          )
+
+          add_targeted_arel(
+            lambda do |inventory_collection|
+              manager_uuids = inventory_collection.parent_inventory_collections.flat_map { |c| c.manager_uuids.to_a }
+              inventory_collection.parent.load_balancer_pool_members
+                .joins(:load_balancer_pool_member_pools => [:load_balancer_pool => :load_balancers])
+                .where(:load_balancer_pool_member_pools => {
+                         'load_balancer_pools' => {
+                           'load_balancers' => {
+                             :ems_ref => manager_uuids
+                           }
+                         }
+                       }).distinct
+            end
+          )
+
+          shared_builder_params
+        end
+
+        def load_balancer_pool_member_pools
+          add_properties(
+            :manager_ref                  => %i(load_balancer_pool load_balancer_pool_member),
+            :parent_inventory_collections => %i(load_balancers)
+          )
+
+          add_targeted_arel(
+            lambda do |inventory_collection|
+              manager_uuids = inventory_collection.parent_inventory_collections.flat_map { |c| c.manager_uuids.to_a }
+              inventory_collection.parent.load_balancer_pool_member_pools
+                .joins(:load_balancer_pool => :load_balancers)
+                .where(:load_balancer_pools => { 'load_balancers' => { :ems_ref => manager_uuids } })
+                .distinct
+            end
+          )
+        end
+
+        def load_balancer_listeners
+          add_properties(
+            :use_ar_object                => true,
+            :parent_inventory_collections => %i(load_balancers),
+          )
+
+          add_targeted_arel(
+            lambda do |inventory_collection|
+              manager_uuids = inventory_collection.parent_inventory_collections.flat_map { |c| c.manager_uuids.to_a }
+              inventory_collection.parent.load_balancer_listeners
+                .joins(:load_balancer)
+                .where(:load_balancers => {:ems_ref => manager_uuids})
+                .distinct
+            end
+          )
+
+          shared_builder_params
+        end
+
+        def load_balancer_listener_pools
+          add_properties(
+            :manager_ref                  => %i(load_balancer_listener load_balancer_pool),
+            :parent_inventory_collections => %i(load_balancers)
+          )
+
+          add_targeted_arel(
+            lambda do |inventory_collection|
+              manager_uuids = inventory_collection.parent_inventory_collections.flat_map { |c| c.manager_uuids.to_a }
+              inventory_collection.parent.load_balancer_listener_pools
+                .joins(:load_balancer_pool => :load_balancers)
+                .where(:load_balancer_pools => {'load_balancers' => {:ems_ref => manager_uuids}})
+                .distinct
+            end
+          )
+        end
+
+        def load_balancer_health_checks
+          add_properties(
+            :parent_inventory_collections => %i(load_balancers)
+          )
+
+          add_targeted_arel(
+            lambda do |inventory_collection|
+              manager_uuids = inventory_collection.parent_inventory_collections.flat_map { |c| c.manager_uuids.to_a }
+              inventory_collection.parent.load_balancer_health_checks
+                .joins(:load_balancer)
+                .where(:load_balancers => {:ems_ref => manager_uuids})
+                .distinct
+            end
+          )
+
+          shared_builder_params
+        end
+
+        def load_balancer_health_check_members
+          add_properties(
+            :manager_ref                  => %i(load_balancer_health_check load_balancer_pool_member),
+            :parent_inventory_collections => %i(load_balancers)
+          )
+
+          add_targeted_arel(
+            lambda do |inventory_collection|
+              manager_uuids = inventory_collection.parent_inventory_collections.flat_map { |c| c.manager_uuids.to_a }
+              inventory_collection.parent.load_balancer_health_check_members
+                .joins(:load_balancer_health_check => :load_balancer)
+                .where(:load_balancer_health_checks => {'load_balancers' => {:ems_ref => manager_uuids}})
+                .distinct
+            end
+          )
+        end
+
+        private
+
+        def shared_builder_params
+          add_builder_params(:ems_id => default_ems_id)
+        end
+
+        def default_ems_id
+          ->(persister) { persister.manager.try(:network_manager).try(:id) || persister.manager.id }
+        end
+      end
+    end
+  end
+end
diff --git a/app/models/manager_refresh/inventory_collection/builder/shared.rb b/app/models/manager_refresh/inventory_collection/builder/shared.rb
new file mode 100644
index 00000000000..9678cb25383
--- /dev/null
+++ b/app/models/manager_refresh/inventory_collection/builder/shared.rb
@@ -0,0 +1,90 @@
+module ManagerRefresh::InventoryCollection::Builder::Shared
+  extend ActiveSupport::Concern
+
+  included do
+    INVENTORY_RECONNECT_BLOCK = lambda do |inventory_collection, inventory_objects_index, attributes_index|
+      relation = inventory_collection.model_class.where(:ems_id => nil)
+
+      return if relation.count <= 0
+
+      inventory_objects_index.each_slice(100) do |batch|
+        batch_refs = batch.map(&:first)
+        relation.where(inventory_collection.manager_ref.first => batch_refs).each do |record|
+          index = inventory_collection.object_index_with_keys(inventory_collection.manager_ref_to_cols, record)
+
+          # We need to delete the record from the inventory_objects_index
+          # and attributes_index, otherwise it would be sent for create.
+          inventory_object = inventory_objects_index.delete(index)
+          hash             = attributes_index.delete(index)
+
+          record.assign_attributes(hash.except(:id, :type))
+          if !inventory_collection.check_changed? || record.changed?
+            record.save!
+            inventory_collection.store_updated_records(record)
+          end
+
+          inventory_object.id = record.id
+        end
+      end
+    end.freeze
+
+    def vendor
+      ::ManageIQ::Providers::Inflector.provider_name(@persister_class).downcase
+    rescue
+      'unknown'
+    end
+
+    def vms
+      vm_template_shared
+    end
+
+    def miq_templates
+      vm_template_shared
+    end
+
+    def vm_template_shared
+      add_properties(
+        :delete_method          => :disconnect_inv,
+        :attributes_blacklist   => %i(genealogy_parent),
+        :use_ar_object          => true, # Because of raw_power_state setter and hooks are needed for settings user
+        :saver_strategy         => :default,
+        :batch_extra_attributes => %i(power_state state_changed_on previous_state),
+        :custom_reconnect_block => INVENTORY_RECONNECT_BLOCK
+      )
+
+      add_builder_params(
+        :ems_id => ->(persister) { persister.manager.id },
+        :vendor => vendor
+      )
+    end
+
+    def hardwares
+      add_properties(
+        :manager_ref                  => %i(vm_or_template),
+        :parent_inventory_collections => %i(vms miq_templates),
+        :use_ar_object                => true, # TODO(lsmola) just because of default value on cpu_sockets, this can be fixed by separating instances_hardwares and images_hardwares
+      )
+    end
+
+    def operating_systems
+      add_properties(
+        :manager_ref                  => %i(vm_or_template),
+        :parent_inventory_collections => %i(vms miq_templates)
+      )
+    end
+
+    def networks
+      add_properties(
+        :manager_ref                  => %i(hardware description),
+        :parent_inventory_collections => %i(vms)
+      )
+    end
+
+    def disks
+      add_properties(
+        :manager_ref                  => %i(hardware device_name),
+        :parent_inventory_collections => %i(vms)
+      )
+    end
+  end
+end
diff --git a/app/models/manager_refresh/inventory_collection/builder/storage_manager.rb b/app/models/manager_refresh/inventory_collection/builder/storage_manager.rb
new file mode 100644
index 00000000000..1cb0cf808b1
--- /dev/null
+++ b/app/models/manager_refresh/inventory_collection/builder/storage_manager.rb
@@ -0,0 +1,9 @@
+module ManagerRefresh
+  class InventoryCollection
+    class Builder
+      class StorageManager < ::ManagerRefresh::InventoryCollection::Builder
+        # Nothing there
+      end
+    end
+  end
+end
diff --git a/app/models/manager_refresh/inventory_object.rb b/app/models/manager_refresh/inventory_object.rb
index fd355859cf8..d12f7545013 100644
--- a/app/models/manager_refresh/inventory_object.rb
+++ b/app/models/manager_refresh/inventory_object.rb
@@ -153,16 +153,23 @@ def dependency?
     end
 
     # Adds setters and getters based on :inventory_object_attributes kwarg passed into InventoryCollection
+    # Methods already defined should not be redefined (causes unexpected behaviour)
     #
     # @param inventory_object_attributes [Array<Symbol>]
     def self.add_attributes(inventory_object_attributes)
+      defined_methods = ManagerRefresh::InventoryObject.instance_methods(false)
+
       inventory_object_attributes.each do |attr|
-        define_method("#{attr}=") do |value|
-          data[attr] = value
+        unless defined_methods.include?("#{attr}=".to_sym)
+          define_method("#{attr}=") do |value|
+            data[attr] = value
+          end
         end
 
-        define_method(attr) do
-          data[attr]
+        unless defined_methods.include?(attr.to_sym)
+          define_method(attr) do
+            data[attr]
+          end
         end
       end
     end

From 61cee7ca3bd1ccf7830bf3379e3f433e1d9853be Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Fri, 27 Apr 2018 14:27:28 +0200
Subject: [PATCH 02/18] Tests for inventory collection builder

Also with fixes discovered by tests
---
 .../inventory_collection/builder.rb           |  36 +--
 .../inventory_collection/builder_spec.rb      | 209 ++++++++++++++++++
 2 files changed, 230 insertions(+), 15 deletions(-)
 create mode 100644 spec/models/manager_refresh/inventory_collection/builder_spec.rb

diff --git a/app/models/manager_refresh/inventory_collection/builder.rb b/app/models/manager_refresh/inventory_collection/builder.rb
index 0cce59bc23e..e729742c880 100644
--- a/app/models/manager_refresh/inventory_collection/builder.rb
+++ b/app/models/manager_refresh/inventory_collection/builder.rb
@@ -1,8 +1,7 @@
 module ManagerRefresh
   class InventoryCollection
     class Builder
-      class MissingModelClassError < StandardError
-      end
+      class MissingModelClassError < StandardError; end
 
       require_nested :CloudManager
       require_nested :NetworkManager
@@ -20,7 +19,7 @@ class MissingModelClassError < StandardError
       #     - @see method auto_model_class
       #   :without_model_class
       #     - if false and no model_class derived or specified, throws exception
-      #   :adv_settings
+      #   :adv_settings_enabled
       #     - values from Advanced settings (doesn't overwrite values specified in code)
       #     - see persister options() method
       def self.default_options
@@ -28,7 +27,7 @@ def self.default_options
           :auto_object_attributes => true,
           :auto_model_class       => true,
           :without_model_class    => false,
-          :adv_settings           => true
+          :adv_settings_enabled   => true
         }
       end
 
@@ -38,9 +37,13 @@ def self.default_options
       #        (optional) method with this name also used for concrete inventory collection specific properties
       # @param persister_class [Class] used for "guessing" model_class
       # @param options [Hash]
-      def self.prepare_data(name, persister_class, options = default_options, &block)
+      def self.prepare_data(name, persister_class, options = {})
+        options = default_options.merge(options)
         builder = new(name, persister_class, options)
-        builder.construct_data(&block)
+        builder.construct_data
+
+        yield(builder) if block_given?
+
         builder
       end
 
@@ -66,13 +69,11 @@ def construct_data
         add_properties(:association => @name)
         add_properties(:model_class => auto_model_class) if @options[:auto_model_class]
 
-        add_properties(@adv_settings, :missing)
+        add_properties(@adv_settings, :missing) if @options[:adv_settings_enabled]
         add_properties(@shared_properties, :missing)
 
         send(@name.to_sym) if respond_to?(@name.to_sym)
 
-        yield(self) if block_given?
-
         add_inventory_attributes(auto_object_attributes) if @options[:auto_object_attributes]
       end
 
@@ -123,7 +124,7 @@ def remove_inventory_attributes(array)
       end
 
       # Clears all inventory object attributes
-      def clear_attributes!
+      def clear_inventory_attributes!
         @inventory_object_attributes = []
       end
 
@@ -198,12 +199,17 @@ def merge_hashes(source, dest, mode)
       #
       # @return [Class | nil] when class doesn't exist, returns nil
       def auto_model_class
-        # a) Provider specific class
-        provider_module = ManageIQ::Providers::Inflector.provider_module(@persister_class).name
-        manager_module = self.class.name.split('::').last
+        model_class = begin
+          # a) Provider specific class
+          provider_module = ManageIQ::Providers::Inflector.provider_module(@persister_class).name
+          manager_module = self.class.name.split('::').last
+
+          class_name = "#{provider_module}::#{manager_module}::#{@name.to_s.classify}"
+          class_name.safe_constantize
+        rescue ::ManageIQ::Providers::Inflector::ObjectNotNamespacedError
+          nil
+        end
 
-        class_name = "#{provider_module}::#{manager_module}::#{@name.to_s.classify}"
-        model_class = class_name.safe_constantize
         if model_class
           model_class
         else
diff --git a/spec/models/manager_refresh/inventory_collection/builder_spec.rb b/spec/models/manager_refresh/inventory_collection/builder_spec.rb
new file mode 100644
index 00000000000..8873d1c5013
--- /dev/null
+++ b/spec/models/manager_refresh/inventory_collection/builder_spec.rb
@@ -0,0 +1,209 @@
+require_relative '../persister/test_persister'
+
+describe ManagerRefresh::InventoryCollection::Builder do
+  before :each do
+    @zone = FactoryGirl.create(:zone)
+    @ems  = FactoryGirl.create(:ems_cloud,
+                               :zone            => @zone,
+                               :network_manager => FactoryGirl.create(:ems_network, :zone => @zone))
+  end
+
+  def create_persister
+    TestPersister.new(@ems, ManagerRefresh::TargetCollection.new(:manager => @ems))
+  end
+
+  let(:persister_class) { ::ManagerRefresh::Inventory::Persister }
+
+  let(:persister) { create_persister }
+
+  let(:adv_settings) { {:strategy => :local_db_find_missing_references, :saver_strategy => :concurrent_safe_batch} }
+
+  def cloud
+    ::ManagerRefresh::InventoryCollection::Builder::CloudManager
+  end
+
+  before :each do
+  end
+
+  # association
+
+  it 'assigns association automatically to InventoryCollection' do
+    ic = cloud.prepare_data(:vms, persister_class).to_inventory_collection
+
+    expect(ic.association).to eq :vms
+  end
+
+  # model_class
+
+  # TODO: move to amazon spec
+  it "derives existing model_class from persister's class" do
+  end
+
+  it "derives existing model_class without persister's class" do
+    data = cloud.prepare_data(:vms, persister_class).to_hash
+
+    expect(data[:model_class]).to eq ::Vm
+  end
+
+  it "replaces derived model_class if model_class defined manually" do
+    data = cloud.prepare_data(:xxx, persister_class) do |builder|
+      builder.add_properties(:model_class => ::Vm)
+    end.to_hash
+
+    expect(data[:model_class]).to eq ::Vm
+  end
+
+  it "doesn't try to derive model_class when disabled" do
+    data = cloud.prepare_data(:vms, persister_class, :auto_model_class => false).to_hash
+
+    expect(data[:model_class]).to be_nil
+  end
+
+  it 'throws exception if model_class not specified' do
+    builder = cloud.prepare_data(:vms, persister_class, :auto_model_class => false)
+
+    expect { builder.to_inventory_collection }.to raise_error(::ManagerRefresh::InventoryCollection::Builder::MissingModelClassError)
+  end
+
+  # Adv settings
+
+  it 'works without Advanced settings' do
+    builder = cloud.prepare_data(:vms, persister_class, :adv_settings_enabled => false)
+
+    expect { builder.to_inventory_collection }.not_to raise_error
+  end
+
+  it 'assigns Advanced settings' do
+    builder = cloud.prepare_data(:tmp, persister_class, :adv_settings => adv_settings)
+    data = builder.to_hash
+
+    expect(data[:strategy]).to eq :local_db_find_missing_references
+    expect(data[:saver_strategy]).to eq :concurrent_safe_batch
+  end
+
+  it "doesn't overwrite defined properties by Advanced settings" do
+    data = cloud.prepare_data(:vms, persister_class, :adv_settings => adv_settings) do |builder|
+      builder.add_properties(:strategy => :custom)
+    end.to_hash
+
+    expect(data[:strategy]).to eq :custom
+    expect(data[:saver_strategy]).to eq :default
+  end
+
+  # Shared properties
+
+  it 'applies shared properties' do
+    data = cloud.prepare_data(:tmp, persister_class, :shared_properties => {:uuid => 1}).to_hash
+
+    expect(data[:uuid]).to eq 1
+  end
+
+  it "doesn't overwrite defined properties by shared properties" do
+    data = cloud.prepare_data(:tmp, persister_class, :shared_properties => {:uuid => 1}) do |builder|
+      builder.add_properties(:uuid => 2)
+    end.to_hash
+
+    expect(data[:uuid]).to eq 2
+  end
+
+  # Properties
+
+  it 'adds properties with add_properties repeatedly' do
+    data = cloud.prepare_data(:tmp, persister_class) do |builder|
+      builder.add_properties(:first => 1, :second => 2)
+      builder.add_properties(:third => 3)
+    end.to_hash
+
+    expect(data[:first]).to eq 1
+    expect(data[:second]).to eq 2
+    expect(data[:third]).to eq 3
+  end
+
+  it 'overrides properties in :overwrite mode' do
+    data = cloud.prepare_data(:tmp, persister_class) do |builder|
+      builder.add_properties(:param => 1)
+      builder.add_properties({:param => 2}, :overwrite)
+    end.to_hash
+
+    expect(data[:param]).to eq 2
+  end
+
+  it "doesn't override properties in :missing mode" do
+    data = cloud.prepare_data(:tmp, persister_class) do |builder|
+      builder.add_properties(:param => 1)
+      builder.add_properties({:param => 2}, :missing)
+    end.to_hash
+
+    expect(data[:param]).to eq 1
+  end
+
+  it 'adds property by method_missing' do
+    data = cloud.prepare_data(:tmp, persister_class) do |builder|
+      builder.add_some_tmp_param(:some_value)
+    end.to_hash
+
+    expect(data[:some_tmp_param]).to eq :some_value
+  end
+
+  # Builder params
+
+  it 'adds builder_params repeatedly' do
+    data = cloud.prepare_data(:tmp, persister_class) do |builder|
+      builder.add_builder_params(:ems_id => 10)
+      builder.add_builder_params(:ems_id => 20)
+    end.to_hash
+
+    expect(data[:builder_params][:ems_id]).to eq 20
+  end
+
+  it 'transforms lambdas in builder_params' do
+    bldr = cloud.prepare_data(:tmp, persister_class) do |builder|
+      builder.add_builder_params(:ems_id => ->(persister) { persister.manager.id })
+    end
+    bldr.transform_builder_params(persister)
+
+    data = bldr.to_hash
+
+    expect(data[:builder_params][:ems_id]).to eq(persister.manager.id)
+  end
+
+  # Inventory object attributes
+
+  it 'derives inventory object attributes automatically' do
+    data = cloud.prepare_data(:vms, persister_class).to_hash
+
+    expect(data[:inventory_object_attributes]).not_to be_empty
+  end
+
+  it "doesn't derive inventory_object_attributes automatically when disabled" do
+    data = cloud.prepare_data(:vms, persister_class, :auto_object_attributes => false).to_hash
+
+    expect(data[:inventory_object_attributes]).to be_empty
+  end
+
+  it 'can add inventory_object_attributes manually' do
+    data = cloud.prepare_data(:tmp, persister_class) do |builder|
+      builder.add_inventory_attributes(%i(attr1 attr2 attr3))
+    end.to_hash
+
+    expect(data[:inventory_object_attributes]).to match_array(%i(attr1 attr2 attr3))
+  end
+
+  it 'can remove inventory_object_attributes' do
+    data = cloud.prepare_data(:tmp, persister_class) do |builder|
+      builder.add_inventory_attributes(%i(attr1 attr2 attr3))
+      builder.remove_inventory_attributes(%i(attr2))
+    end.to_hash
+
+    expect(data[:inventory_object_attributes]).to match_array(%i(attr1 attr3))
+  end
+
+  it 'can clear all inventory_object_attributes' do
+    data = cloud.prepare_data(:vms, persister_class) do |builder|
+      builder.add_inventory_attributes(%i(attr1 attr2 attr3))
+      builder.clear_inventory_attributes!
+    end.to_hash
+
+    expect(data[:inventory_object_attributes]).to be_empty
+  end
+end

From ce7bb7da898dea410c5ebd1e33df93503500a482 Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Wed, 2 May 2018 11:34:22 +0200
Subject: [PATCH 03/18] Comments and :missing => :if_missing option

---
 .../manager_refresh/inventory/persister.rb    | 16 ++++---
 .../inventory_collection/builder.rb           | 42 +++++++++++--------
 .../inventory_collection/builder_spec.rb      | 40 ++++++++----------
 3 files changed, 52 insertions(+), 46 deletions(-)

diff --git a/app/models/manager_refresh/inventory/persister.rb b/app/models/manager_refresh/inventory/persister.rb
index 299a601bcf4..a5334d2307f 100644
--- a/app/models/manager_refresh/inventory/persister.rb
+++ b/app/models/manager_refresh/inventory/persister.rb
@@ -125,10 +125,11 @@ def shared_options
 
   # Interface for creating InventoryCollection under @collections
   #
-  # @param builder_class [ManagerRefresh::InventoryCollection::Builder]
-  # @param collection_name [Symbol] used as InventoryCollection:association
-  # @param extra_properties [Hash] props from InventoryCollection.initialize list
-  #         - overwrites all previous settings
+  # @param builder_class    [ManagerRefresh::InventoryCollection::Builder] or subclasses
+  # @param collection_name  [Symbol] used as InventoryCollection:association
+  # @param extra_properties [Hash]   props from InventoryCollection.initialize list
+  #         - adds/overwrites properties added by builder
+  #
   # @param settings [Hash] builder settings
   #         - @see ManagerRefresh::InventoryCollection::Builder.default_options
   #         - @see make_builder_settings()
@@ -139,6 +140,9 @@ def shared_options
   #       :strategy => :local_db_cache_all,
   #     )
   #   )
+  #
+  # @see ManagerRefresh::InventoryCollection::Builder
+  #
   def add_collection(builder_class, collection_name, extra_properties = {}, settings = {}, &block)
     builder = builder_class.prepare_data(collection_name,
                                          self.class,
@@ -146,9 +150,9 @@ def add_collection(builder_class, collection_name, extra_properties = {}, settin
                                          &block)
 
     builder.add_properties(extra_properties) if extra_properties.present?
-    builder.add_properties({:parent => manager}, :missing) if manager.present?
+    builder.add_properties({:parent => manager}, :if_missing) if manager.present?
 
-    builder.transform_builder_params(self)
+    builder.evaluate_lambdas!(self)
 
     collections[collection_name] = builder.to_inventory_collection
   end
diff --git a/app/models/manager_refresh/inventory_collection/builder.rb b/app/models/manager_refresh/inventory_collection/builder.rb
index e729742c880..c0ab6e8c4b0 100644
--- a/app/models/manager_refresh/inventory_collection/builder.rb
+++ b/app/models/manager_refresh/inventory_collection/builder.rb
@@ -1,5 +1,6 @@
 module ManagerRefresh
   class InventoryCollection
+    # @see test in /spec/models/manager_refresh/inventory_collection/builder_spec.rb
     class Builder
       class MissingModelClassError < StandardError; end
 
@@ -69,8 +70,8 @@ def construct_data
         add_properties(:association => @name)
         add_properties(:model_class => auto_model_class) if @options[:auto_model_class]
 
-        add_properties(@adv_settings, :missing) if @options[:adv_settings_enabled]
-        add_properties(@shared_properties, :missing)
+        add_properties(@adv_settings, :if_missing) if @options[:adv_settings_enabled]
+        add_properties(@shared_properties, :if_missing)
 
         send(@name.to_sym) if respond_to?(@name.to_sym)
 
@@ -87,8 +88,10 @@ def to_inventory_collection
       end
 
       #
-      # Missing method *add_some_property(value)* converted to:
-      # add_properties(:some_property => value)
+      # Missing method
+      #   - add_some_property(value)
+      # converted to:
+      #   - add_properties(:some_property => value)
       #
       def method_missing(method_name, *arguments, &block)
         if method_name.to_s.starts_with?('add_')
@@ -108,7 +111,7 @@ def respond_to_missing?(method_name, _include_private = false)
       # @see ManagerRefresh::InventoryCollection.initialize for list of properties
       #
       # @param props [Hash]
-      # @param mode [Symbol] :overwrite | :missing
+      # @param mode [Symbol] :overwrite | :if_missing
       def add_properties(props = {}, mode = :overwrite)
         @properties = merge_hashes(@properties, props, mode)
       end
@@ -133,17 +136,9 @@ def add_builder_params(params = {}, mode = :overwrite)
         @builder_params = merge_hashes(@builder_params, params, mode)
       end
 
-      # Evaluates lambda blocks in @builder_params
-      def transform_builder_params(persister)
-        if @builder_params
-          @builder_params = @builder_params.transform_values do |value|
-            if value.respond_to?(:call)
-              value.call(persister)
-            else
-              value
-            end
-          end
-        end
+      # Evaluates lambda blocks
+      def evaluate_lambdas!(persister)
+        evaluate_builder_params_lambdas!(persister)
       end
 
       # Adds key/values to dependency_attributes (part of @properties)
@@ -166,7 +161,7 @@ def to_hash
       # - a) all keys from dest (overwrite mode)
       # - b) missing keys (missing mode)
       #
-      # @param mode [Symbol] :overwrite | :missing
+      # @param mode [Symbol] :overwrite | :if_missing
       def merge_hashes(source, dest, mode)
         return source if source.nil? || dest.nil?
 
@@ -229,6 +224,19 @@ def auto_object_attributes
           setter.to_s[0..setter.length - 2].to_sym
         end
       end
+
+      # Evaluates lambda blocks in @builder_params
+      def evaluate_builder_params_lambdas!(persister)
+        if @builder_params
+          @builder_params = @builder_params.transform_values do |value|
+            if value.respond_to?(:call)
+              value.call(persister)
+            else
+              value
+            end
+          end
+        end
+      end
     end
   end
 end
diff --git a/spec/models/manager_refresh/inventory_collection/builder_spec.rb b/spec/models/manager_refresh/inventory_collection/builder_spec.rb
index 8873d1c5013..75eac6b00b2 100644
--- a/spec/models/manager_refresh/inventory_collection/builder_spec.rb
+++ b/spec/models/manager_refresh/inventory_collection/builder_spec.rb
@@ -6,26 +6,20 @@
     @ems  = FactoryGirl.create(:ems_cloud,
                                :zone            => @zone,
                                :network_manager => FactoryGirl.create(:ems_network, :zone => @zone))
+    @persister = create_persister
   end
 
   def create_persister
     TestPersister.new(@ems, ManagerRefresh::TargetCollection.new(:manager => @ems))
   end
 
-  let(:persister_class) { ::ManagerRefresh::Inventory::Persister }
-
-  let(:persister) { create_persister }
-
   let(:adv_settings) { {:strategy => :local_db_find_missing_references, :saver_strategy => :concurrent_safe_batch} }
 
-  def cloud
-    ::ManagerRefresh::InventoryCollection::Builder::CloudManager
-  end
+  let(:cloud) { ::ManagerRefresh::InventoryCollection::Builder::CloudManager }
 
-  before :each do
-  end
+  let(:persister_class) { ::ManagerRefresh::Inventory::Persister }
 
-  # association
+  # --- association ---
 
   it 'assigns association automatically to InventoryCollection' do
     ic = cloud.prepare_data(:vms, persister_class).to_inventory_collection
@@ -33,7 +27,7 @@ def cloud
     expect(ic.association).to eq :vms
   end
 
-  # model_class
+  # --- model_class ---
 
   # TODO: move to amazon spec
   it "derives existing model_class from persister's class" do
@@ -46,11 +40,11 @@ def cloud
   end
 
   it "replaces derived model_class if model_class defined manually" do
-    data = cloud.prepare_data(:xxx, persister_class) do |builder|
-      builder.add_properties(:model_class => ::Vm)
+    data = cloud.prepare_data(:vms, persister_class) do |builder|
+      builder.add_properties(:model_class => ::MiqTemplate)
     end.to_hash
 
-    expect(data[:model_class]).to eq ::Vm
+    expect(data[:model_class]).to eq ::MiqTemplate
   end
 
   it "doesn't try to derive model_class when disabled" do
@@ -65,7 +59,7 @@ def cloud
     expect { builder.to_inventory_collection }.to raise_error(::ManagerRefresh::InventoryCollection::Builder::MissingModelClassError)
   end
 
-  # Adv settings
+  # --- adv. settings (TODO: link to gui)---
 
   it 'works without Advanced settings' do
     builder = cloud.prepare_data(:vms, persister_class, :adv_settings_enabled => false)
@@ -90,7 +84,7 @@ def cloud
     expect(data[:saver_strategy]).to eq :default
   end
 
-  # Shared properties
+  # --- shared properties ---
 
   it 'applies shared properties' do
     data = cloud.prepare_data(:tmp, persister_class, :shared_properties => {:uuid => 1}).to_hash
@@ -106,7 +100,7 @@ def cloud
     expect(data[:uuid]).to eq 2
   end
 
-  # Properties
+  # --- properties ---
 
   it 'adds properties with add_properties repeatedly' do
     data = cloud.prepare_data(:tmp, persister_class) do |builder|
@@ -128,10 +122,10 @@ def cloud
     expect(data[:param]).to eq 2
   end
 
-  it "doesn't override properties in :missing mode" do
+  it "doesn't override properties in :if_missing mode" do
     data = cloud.prepare_data(:tmp, persister_class) do |builder|
       builder.add_properties(:param => 1)
-      builder.add_properties({:param => 2}, :missing)
+      builder.add_properties({:param => 2}, :if_missing)
     end.to_hash
 
     expect(data[:param]).to eq 1
@@ -145,7 +139,7 @@ def cloud
     expect(data[:some_tmp_param]).to eq :some_value
   end
 
-  # Builder params
+  # --- builder params ---
 
   it 'adds builder_params repeatedly' do
     data = cloud.prepare_data(:tmp, persister_class) do |builder|
@@ -160,14 +154,14 @@ def cloud
     bldr = cloud.prepare_data(:tmp, persister_class) do |builder|
       builder.add_builder_params(:ems_id => ->(persister) { persister.manager.id })
     end
-    bldr.transform_builder_params(persister)
+    bldr.evaluate_lambdas!(@persister)
 
     data = bldr.to_hash
 
-    expect(data[:builder_params][:ems_id]).to eq(persister.manager.id)
+    expect(data[:builder_params][:ems_id]).to eq(@persister.manager.id)
   end
 
-  # Inventory object attributes
+  # --- inventory object attributes ---
 
   it 'derives inventory object attributes automatically' do
     data = cloud.prepare_data(:vms, persister_class).to_hash

From 9974deca768c0e4e5cb43dd443f5ba3aa457ed60 Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Thu, 3 May 2018 11:48:00 +0200
Subject: [PATCH 04/18] Added automatic :manager_uuids to InventoryCollection

Also moved targeted?, references() and name_references() to base class
---
 app/models/manager_refresh/inventory/persister.rb | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/app/models/manager_refresh/inventory/persister.rb b/app/models/manager_refresh/inventory/persister.rb
index a5334d2307f..cd50e118a6e 100644
--- a/app/models/manager_refresh/inventory/persister.rb
+++ b/app/models/manager_refresh/inventory/persister.rb
@@ -117,6 +117,10 @@ def initialize_inventory_collections
     # can be implemented in a subclass
   end
 
+  def targeted?
+    false
+  end
+
   # @return [Hash] kwargs shared for all InventoryCollection objects
   def shared_options
     # can be implemented in a subclass
@@ -150,6 +154,9 @@ def add_collection(builder_class, collection_name, extra_properties = {}, settin
                                          &block)
 
     builder.add_properties(extra_properties) if extra_properties.present?
+
+    builder.add_properties({:manager_uuids => references(collection_name)}, :if_missing) if targeted?
+
     builder.add_properties({:parent => manager}, :if_missing) if manager.present?
 
     builder.evaluate_lambdas!(self)
@@ -230,6 +237,14 @@ def to_hash
     }
   end
 
+  def references(collection)
+    target.manager_refs_by_association.try(:[], collection).try(:[], :ems_ref).try(:to_a) || []
+  end
+
+  def name_references(collection)
+    target.manager_refs_by_association.try(:[], collection).try(:[], :name).try(:to_a) || []
+  end
+
   class << self
     protected
 

From 14ae225646e90ed17d05ce37b8dc34fc4cfe1691 Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Thu, 3 May 2018 19:41:03 +0200
Subject: [PATCH 05/18] Automatic Network ICs parent for TargetedCollection
 persister

---
 app/models/manager_refresh/inventory/persister.rb     |  2 +-
 .../manager_refresh/inventory_collection/builder.rb   | 11 +++++++++++
 .../inventory_collection/builder/network_manager.rb   |  2 +-
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/app/models/manager_refresh/inventory/persister.rb b/app/models/manager_refresh/inventory/persister.rb
index cd50e118a6e..34077e2ce00 100644
--- a/app/models/manager_refresh/inventory/persister.rb
+++ b/app/models/manager_refresh/inventory/persister.rb
@@ -157,7 +157,7 @@ def add_collection(builder_class, collection_name, extra_properties = {}, settin
 
     builder.add_properties({:manager_uuids => references(collection_name)}, :if_missing) if targeted?
 
-    builder.add_properties({:parent => manager}, :if_missing) if manager.present?
+    builder.add_parent_if_missing(manager, targeted?) if manager.present?
 
     builder.evaluate_lambdas!(self)
 
diff --git a/app/models/manager_refresh/inventory_collection/builder.rb b/app/models/manager_refresh/inventory_collection/builder.rb
index c0ab6e8c4b0..75a8e7f2146 100644
--- a/app/models/manager_refresh/inventory_collection/builder.rb
+++ b/app/models/manager_refresh/inventory_collection/builder.rb
@@ -155,6 +155,17 @@ def to_hash
         )
       end
 
+      # Adds parent to InventoryCollection if not set before
+      def add_parent_if_missing(manager, targeted)
+        if manager.present?
+          if targeted && self.class.to_s.split('::').last.starts_with?('Network')
+            add_properties({:parent => manager.network_manager}, :if_missing)
+          else
+            add_properties({:parent => manager}, :if_missing)
+          end
+        end
+      end
+
       protected
 
       # Extends source hash with
diff --git a/app/models/manager_refresh/inventory_collection/builder/network_manager.rb b/app/models/manager_refresh/inventory_collection/builder/network_manager.rb
index 07849056078..0988b16e13b 100644
--- a/app/models/manager_refresh/inventory_collection/builder/network_manager.rb
+++ b/app/models/manager_refresh/inventory_collection/builder/network_manager.rb
@@ -4,7 +4,7 @@ class Builder
       class NetworkManager < ::ManagerRefresh::InventoryCollection::Builder
         def cloud_subnet_network_ports
           add_properties(
-            :model_class                  => ::CloudSubnetNetworkPort,
+            # :model_class                  => ::CloudSubnetNetworkPort,
             :manager_ref                  => %i(address cloud_subnet network_port),
             :parent_inventory_collections => %i(vms network_ports load_balancers)
           )

From 05464a71e97f43a0b4ee478a174350f872abf13b Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Fri, 4 May 2018 13:30:12 +0200
Subject: [PATCH 06/18] IC Builder for AnsibleTower

For AutomationManager's persister
---
 .../builder/automation_manager.rb             | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 app/models/manager_refresh/inventory_collection/builder/automation_manager.rb

diff --git a/app/models/manager_refresh/inventory_collection/builder/automation_manager.rb b/app/models/manager_refresh/inventory_collection/builder/automation_manager.rb
new file mode 100644
index 00000000000..71b5c9df220
--- /dev/null
+++ b/app/models/manager_refresh/inventory_collection/builder/automation_manager.rb
@@ -0,0 +1,58 @@
+module ManagerRefresh
+  class InventoryCollection
+    class Builder
+      # TODO: (mslemr) Remove /manager_refresh/inventory/[core, automation_manager, cloud_manager, ?middleware_manager?].rb
+      # TODO: (mslemr) think about lib/generators/provider/templates/app/models/manageiq/providers/%provider_name%/inventory/persister/cloud_manager.rb
+      class AutomationManager < ::ManagerRefresh::InventoryCollection::Builder
+        def configuration_scripts
+          default_manager_ref
+          default_builder_params
+        end
+
+        def configuration_script_payloads
+          add_properties(
+            :manager_ref => %i(configuration_script_source manager_ref)
+          )
+          default_builder_params
+        end
+
+        def configuration_script_sources
+          default_manager_ref
+          default_builder_params
+        end
+
+        def configured_systems
+          default_manager_ref
+          default_builder_params
+        end
+
+        def credentials
+          default_manager_ref
+          add_builder_params(
+            :resource => ->(persister) { persister.manager }
+          )
+        end
+
+        def inventory_root_groups
+          default_builder_params
+        end
+
+        def vms
+          add_properties(:manager_ref => %i(uid_ems))
+        end
+
+        protected
+
+        def default_manager_ref
+          add_properties(:manager_ref => %i(manager_ref))
+        end
+
+        def default_builder_params
+          add_builder_params(
+            :manager => ->(persister) { persister.manager }
+          )
+        end
+      end
+    end
+  end
+end

From 116770d9772662d4df8b80e3c16652be28a740a1 Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Fri, 4 May 2018 16:43:31 +0200
Subject: [PATCH 07/18] Improved automatic parent assignment

Fix for managers who doesn't respond to :network_manager (nuage)
And option to disable this
---
 app/models/manager_refresh/inventory_collection/builder.rb | 7 ++++++-
 .../inventory_collection/builder/network_manager.rb        | 7 ++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/app/models/manager_refresh/inventory_collection/builder.rb b/app/models/manager_refresh/inventory_collection/builder.rb
index 75a8e7f2146..d0991c63136 100644
--- a/app/models/manager_refresh/inventory_collection/builder.rb
+++ b/app/models/manager_refresh/inventory_collection/builder.rb
@@ -158,7 +158,7 @@ def to_hash
       # Adds parent to InventoryCollection if not set before
       def add_parent_if_missing(manager, targeted)
         if manager.present?
-          if targeted && self.class.to_s.split('::').last.starts_with?('Network')
+          if targeted && network_manager_collections? && manager.respond_to?(:network_manager)
             add_properties({:parent => manager.network_manager}, :if_missing)
           else
             add_properties({:parent => manager}, :if_missing)
@@ -248,6 +248,11 @@ def evaluate_builder_params_lambdas!(persister)
           end
         end
       end
+
+      # InventoryCollection definitions for NetworkManager?
+      def network_manager_collections?
+        false
+      end
     end
   end
 end
diff --git a/app/models/manager_refresh/inventory_collection/builder/network_manager.rb b/app/models/manager_refresh/inventory_collection/builder/network_manager.rb
index 0988b16e13b..3106935a35f 100644
--- a/app/models/manager_refresh/inventory_collection/builder/network_manager.rb
+++ b/app/models/manager_refresh/inventory_collection/builder/network_manager.rb
@@ -193,7 +193,7 @@ def load_balancer_health_check_members
           )
         end
 
-        private
+        protected
 
         def shared_builder_params
           add_builder_params(:ems_id => default_ems_id)
@@ -202,6 +202,11 @@ def shared_builder_params
         def default_ems_id
           ->(persister) { persister.manager.try(:network_manager).try(:id) || persister.manager.id }
         end
+
+        # InventoryCollection definitions for NetworkManager?
+        def network_manager_collections?
+          true
+        end
       end
     end
   end

From 4bf44a6a1bba628a0c0a90473bac7627f8e8902f Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Wed, 9 May 2018 14:47:41 +0200
Subject: [PATCH 08/18] IC Builder automatic parent optional

Added option to disable automatic parent assignment + tests
---
 .../inventory_collection/builder.rb           | 31 ++++++++---
 .../inventory_collection/builder_spec.rb      | 55 +++++++++++++++++++
 2 files changed, 77 insertions(+), 9 deletions(-)

diff --git a/app/models/manager_refresh/inventory_collection/builder.rb b/app/models/manager_refresh/inventory_collection/builder.rb
index d0991c63136..90a961c0c14 100644
--- a/app/models/manager_refresh/inventory_collection/builder.rb
+++ b/app/models/manager_refresh/inventory_collection/builder.rb
@@ -12,23 +12,33 @@ class MissingModelClassError < StandardError; end
       include ::ManagerRefresh::InventoryCollection::Builder::Shared
 
       # Default options for builder
-      #   :auto_object_attributes
-      #     - auto creates inventory_object_attributes from target model_class setters
-      #     - attributes used in InventoryObject.add_attributes
+      #   :adv_settings
+      #     - values from Advanced settings (doesn't overwrite values specified in code)
+      #     - @see method ManagerRefresh::Inventory::Persister.options()
+      #   :adv_settings_enabled
+      #     - enable/disable this properties
+      #   :auto_missing_parent
+      #     - auto assigns parent if this property is missing
+      #     - @see method add_parent_if_missing()
       #   :auto_model_class
       #     - tries to set model_class from persister class
       #     - @see method auto_model_class
+      #   :auto_object_attributes
+      #     - auto creates inventory_object_attributes from target model_class setters
+      #     - attributes used in InventoryObject.add_attributes
+      #   :shared_properties
+      #     - any properties applied if missing (not explicitly specified)
       #   :without_model_class
       #     - if false and no model_class derived or specified, throws exception
-      #   :adv_settings_enabled
-      #     - values from Advanced settings (doesn't overwrite values specified in code)
-      #     - see persister options() method
       def self.default_options
         {
-          :auto_object_attributes => true,
+          :adv_settings           => {},
+          :adv_settings_enabled   => true,
+          :auto_missing_parent    => true,
           :auto_model_class       => true,
+          :auto_object_attributes => true,
+          :shared_properties      => {},
           :without_model_class    => false,
-          :adv_settings_enabled   => true
         }
       end
 
@@ -156,7 +166,10 @@ def to_hash
       end
 
       # Adds parent to InventoryCollection if not set before
-      def add_parent_if_missing(manager, targeted)
+      # Can be disabled by :auto_missing_parent => false
+      def add_parent_if_missing(manager, targeted = false)
+        return unless @options[:auto_missing_parent]
+
         if manager.present?
           if targeted && network_manager_collections? && manager.respond_to?(:network_manager)
             add_properties({:parent => manager.network_manager}, :if_missing)
diff --git a/spec/models/manager_refresh/inventory_collection/builder_spec.rb b/spec/models/manager_refresh/inventory_collection/builder_spec.rb
index 75eac6b00b2..6a628ff32ca 100644
--- a/spec/models/manager_refresh/inventory_collection/builder_spec.rb
+++ b/spec/models/manager_refresh/inventory_collection/builder_spec.rb
@@ -17,6 +17,8 @@ def create_persister
 
   let(:cloud) { ::ManagerRefresh::InventoryCollection::Builder::CloudManager }
 
+  let(:network) { ::ManagerRefresh::InventoryCollection::Builder::NetworkManager }
+
   let(:persister_class) { ::ManagerRefresh::Inventory::Persister }
 
   # --- association ---
@@ -145,9 +147,11 @@ def create_persister
     data = cloud.prepare_data(:tmp, persister_class) do |builder|
       builder.add_builder_params(:ems_id => 10)
       builder.add_builder_params(:ems_id => 20)
+      builder.add_builder_params(:tmp_id => 30)
     end.to_hash
 
     expect(data[:builder_params][:ems_id]).to eq 20
+    expect(data[:builder_params][:tmp_id]).to eq 30
   end
 
   it 'transforms lambdas in builder_params' do
@@ -200,4 +204,55 @@ def create_persister
 
     expect(data[:inventory_object_attributes]).to be_empty
   end
+
+  # --- parent ---
+
+  it 'assigns parent to ems automatically if not set' do
+    builder = cloud.prepare_data(:tmp, persister_class)
+    builder.add_parent_if_missing(@ems)
+    data = builder.to_hash
+
+    expect(data[:parent].id).to eq(@ems.id)
+  end
+
+  it 'does not auto-assign parent if set' do
+    builder = cloud.prepare_data(:tmp, persister_class) do |bld|
+      bld.add_properties(:parent => 'some_parent')
+    end
+    builder.add_parent_if_missing(@ems)
+    data = builder.to_hash
+
+    expect(data[:parent]).to eq('some_parent')
+  end
+
+  it 'does not auto-assign parent if switched off' do
+    builder = cloud.prepare_data(:tmp, persister_class, :auto_missing_parent => false)
+    builder.add_parent_if_missing(@ems)
+
+    data = builder.to_hash
+
+    expect(data[:parent]).to be_nil
+  end
+
+  context 'creating network inv. collections' do
+    it "assigns EMS's network manager as parent when targeted refresh" do
+      builder = network.prepare_data(:tmp, persister_class)
+      builder.add_parent_if_missing(@ems, true)
+      data = builder.to_hash
+
+      expect(data[:parent].id).to eq(@ems.network_manager.id)
+    end
+
+    it "assigns EMS instead of its network manager as parent if EMS does not respond to network_manager" do
+      ems_network = FactoryGirl.create(:ems_nuage_network,
+                                       :zone => @zone)
+      expect(ems_network.respond_to?(:network_manager)).to be_falsy
+
+      builder = network.prepare_data(:tmp, persister_class)
+      builder.add_parent_if_missing(ems_network)
+      data = builder.to_hash
+
+      expect(data[:parent].id).to eq(ems_network.id)
+    end
+  end
 end

From 6fc007c217967db0348c4cf5151bbdec765f08b6 Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Thu, 10 May 2018 16:07:27 +0200
Subject: [PATCH 09/18] Changed :auto_object_attributes =>
 :auto_inventory_attributes

---
 .../inventory_collection/builder.rb           | 22 +++++++++----------
 .../inventory_collection/builder_spec.rb      |  2 +-
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/app/models/manager_refresh/inventory_collection/builder.rb b/app/models/manager_refresh/inventory_collection/builder.rb
index 90a961c0c14..71310ffb4d8 100644
--- a/app/models/manager_refresh/inventory_collection/builder.rb
+++ b/app/models/manager_refresh/inventory_collection/builder.rb
@@ -23,7 +23,7 @@ class MissingModelClassError < StandardError; end
       #   :auto_model_class
       #     - tries to set model_class from persister class
       #     - @see method auto_model_class
-      #   :auto_object_attributes
+      #   :auto_inventory_attributes
       #     - auto creates inventory_object_attributes from target model_class setters
       #     - attributes used in InventoryObject.add_attributes
       #   :shared_properties
@@ -32,13 +32,13 @@ class MissingModelClassError < StandardError; end
       #     - if false and no model_class derived or specified, throws exception
       def self.default_options
         {
-          :adv_settings           => {},
-          :adv_settings_enabled   => true,
-          :auto_missing_parent    => true,
-          :auto_model_class       => true,
-          :auto_object_attributes => true,
-          :shared_properties      => {},
-          :without_model_class    => false,
+          :adv_settings              => {},
+          :adv_settings_enabled      => true,
+          :auto_missing_parent       => true,
+          :auto_model_class          => true,
+          :auto_inventory_attributes => true,
+          :shared_properties         => {},
+          :without_model_class       => false,
         }
       end
 
@@ -85,7 +85,7 @@ def construct_data
 
         send(@name.to_sym) if respond_to?(@name.to_sym)
 
-        add_inventory_attributes(auto_object_attributes) if @options[:auto_object_attributes]
+        add_inventory_attributes(auto_inventory_attributes) if @options[:auto_inventory_attributes]
       end
 
       # Creates InventoryCollection
@@ -239,9 +239,9 @@ def auto_model_class
 
       # Inventory object attributes are derived from setters
       #
-      # Can be disabled by options :auto_object_attributes => false
+      # Can be disabled by options :auto_inventory_attributes => false
       #   - attributes can be manually set via method add_inventory_attributes()
-      def auto_object_attributes
+      def auto_inventory_attributes
         return if @properties[:model_class].nil?
 
         (@properties[:model_class].new.methods - ApplicationRecord.methods).grep(/^[\w]+?\=$/).collect do |setter|
diff --git a/spec/models/manager_refresh/inventory_collection/builder_spec.rb b/spec/models/manager_refresh/inventory_collection/builder_spec.rb
index 6a628ff32ca..a1c90439933 100644
--- a/spec/models/manager_refresh/inventory_collection/builder_spec.rb
+++ b/spec/models/manager_refresh/inventory_collection/builder_spec.rb
@@ -174,7 +174,7 @@ def create_persister
   end
 
   it "doesn't derive inventory_object_attributes automatically when disabled" do
-    data = cloud.prepare_data(:vms, persister_class, :auto_object_attributes => false).to_hash
+    data = cloud.prepare_data(:vms, persister_class, :auto_inventory_attributes => false).to_hash
 
     expect(data[:inventory_object_attributes]).to be_empty
   end

From a124e9759364a2a6898bb90c434b6af78b8a189f Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Thu, 24 May 2018 11:33:18 +0200
Subject: [PATCH 10/18] Fixed infra emc_clusters definition

Datacenter_id inventory attribute have to be added manually
---
 .../builder/infra_manager.rb                  | 40 ++++++++++---------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/app/models/manager_refresh/inventory_collection/builder/infra_manager.rb b/app/models/manager_refresh/inventory_collection/builder/infra_manager.rb
index 6ad63450a64..7728c8091c0 100644
--- a/app/models/manager_refresh/inventory_collection/builder/infra_manager.rb
+++ b/app/models/manager_refresh/inventory_collection/builder/infra_manager.rb
@@ -80,6 +80,8 @@ def ems_clusters
           add_properties(
             :attributes_blacklist => %i(ems_children datacenter_id),
           )
+
+          add_inventory_attributes(%i(datacenter_id))
           shared_builder_params
         end
 
@@ -125,31 +127,33 @@ def hosts
         def vms
           super
 
-          add_custom_reconnect_block(
-            lambda do |inventory_collection, inventory_objects_index, attributes_index|
-              relation = inventory_collection.model_class.where(:ems_id => nil)
+          custom_reconnect_block = lambda do |inventory_collection, inventory_objects_index, attributes_index|
+            relation = inventory_collection.model_class.where(:ems_id => nil)
 
-              return if relation.count <= 0
+            return if relation.count <= 0
 
-              inventory_objects_index.each_slice(100) do |batch|
-                relation.where(inventory_collection.manager_ref.first => batch.map(&:first)).each do |record|
-                  index = inventory_collection.object_index_with_keys(inventory_collection.manager_ref_to_cols, record)
+            inventory_objects_index.each_slice(100) do |batch|
+              relation.where(inventory_collection.manager_ref.first => batch.map(&:first)).each do |record|
+                index = inventory_collection.object_index_with_keys(inventory_collection.manager_ref_to_cols, record)
 
-                  # We need to delete the record from the inventory_objects_index and attributes_index, otherwise it
-                  # would be sent for create.
-                  inventory_object = inventory_objects_index.delete(index)
-                  hash             = attributes_index.delete(index)
+                # We need to delete the record from the inventory_objects_index and attributes_index, otherwise it
+                # would be sent for create.
+                inventory_object = inventory_objects_index.delete(index)
+                hash = attributes_index.delete(index)
 
-                  record.assign_attributes(hash.except(:id, :type))
-                  if !inventory_collection.check_changed? || record.changed?
-                    record.save!
-                    inventory_collection.store_updated_records(record)
-                  end
-
-                  inventory_object.id = record.id
+                record.assign_attributes(hash.except(:id, :type))
+                if !inventory_collection.check_changed? || record.changed?
+                  record.save!
+                  inventory_collection.store_updated_records(record)
                 end
+
+                inventory_object.id = record.id
               end
             end
+          end
+
+          add_properties(
+            :custom_reconnect_block => custom_reconnect_block
           )
         end
 

From 0e48867d5fe32f8ecbf84f71aef4357b51facd59 Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Thu, 24 May 2018 17:06:04 +0200
Subject: [PATCH 11/18] Extracted IC builder methods for persister

Also builder classes definition moved from providers to core
---
 .../manager_refresh/inventory/persister.rb    | 67 +------------
 .../inventory_collection/builder.rb           |  4 +-
 .../builder/persister_helper.rb               | 95 +++++++++++++++++++
 3 files changed, 100 insertions(+), 66 deletions(-)
 create mode 100644 app/models/manager_refresh/inventory_collection/builder/persister_helper.rb

diff --git a/app/models/manager_refresh/inventory/persister.rb b/app/models/manager_refresh/inventory/persister.rb
index 34077e2ce00..2eda7e1909d 100644
--- a/app/models/manager_refresh/inventory/persister.rb
+++ b/app/models/manager_refresh/inventory/persister.rb
@@ -4,6 +4,8 @@ class ManagerRefresh::Inventory::Persister
 
   attr_reader :manager, :target, :collections
 
+  include ::ManagerRefresh::InventoryCollection::Builder::PersisterHelper
+
   # @param manager [ManageIQ::Providers::BaseManager] A manager object
   # @param target [Object] A refresh Target object
   def initialize(manager, target = nil)
@@ -117,63 +119,6 @@ def initialize_inventory_collections
     # can be implemented in a subclass
   end
 
-  def targeted?
-    false
-  end
-
-  # @return [Hash] kwargs shared for all InventoryCollection objects
-  def shared_options
-    # can be implemented in a subclass
-    {}
-  end
-
-  # Interface for creating InventoryCollection under @collections
-  #
-  # @param builder_class    [ManagerRefresh::InventoryCollection::Builder] or subclasses
-  # @param collection_name  [Symbol] used as InventoryCollection:association
-  # @param extra_properties [Hash]   props from InventoryCollection.initialize list
-  #         - adds/overwrites properties added by builder
-  #
-  # @param settings [Hash] builder settings
-  #         - @see ManagerRefresh::InventoryCollection::Builder.default_options
-  #         - @see make_builder_settings()
-  #
-  # @example
-  #   add_collection(ManagerRefresh::InventoryCollection::Builder::CloudManager, :vms) do |builder|
-  #     builder.add_properties(
-  #       :strategy => :local_db_cache_all,
-  #     )
-  #   )
-  #
-  # @see ManagerRefresh::InventoryCollection::Builder
-  #
-  def add_collection(builder_class, collection_name, extra_properties = {}, settings = {}, &block)
-    builder = builder_class.prepare_data(collection_name,
-                                         self.class,
-                                         make_builder_settings(settings),
-                                         &block)
-
-    builder.add_properties(extra_properties) if extra_properties.present?
-
-    builder.add_properties({:manager_uuids => references(collection_name)}, :if_missing) if targeted?
-
-    builder.add_parent_if_missing(manager, targeted?) if manager.present?
-
-    builder.evaluate_lambdas!(self)
-
-    collections[collection_name] = builder.to_inventory_collection
-  end
-
-  # @param extra_settings [Hash]
-  def make_builder_settings(extra_settings = {})
-    opts = ::ManagerRefresh::InventoryCollection::Builder.default_options
-
-    opts[:adv_settings] = options[:inventory_collections].try(:to_hash) || {}
-    opts[:shared_properties] = shared_options
-
-    opts.merge(extra_settings)
-  end
-
   # Adds 1 ManagerRefresh::InventoryCollection under a target.collections using :association key as index
   #
   # @param options [Hash] Hash used for ManagerRefresh::InventoryCollection initialize
@@ -237,14 +182,6 @@ def to_hash
     }
   end
 
-  def references(collection)
-    target.manager_refs_by_association.try(:[], collection).try(:[], :ems_ref).try(:to_a) || []
-  end
-
-  def name_references(collection)
-    target.manager_refs_by_association.try(:[], collection).try(:[], :name).try(:to_a) || []
-  end
-
   class << self
     protected
 
diff --git a/app/models/manager_refresh/inventory_collection/builder.rb b/app/models/manager_refresh/inventory_collection/builder.rb
index 71310ffb4d8..3cc8de00239 100644
--- a/app/models/manager_refresh/inventory_collection/builder.rb
+++ b/app/models/manager_refresh/inventory_collection/builder.rb
@@ -5,9 +5,11 @@ class Builder
       class MissingModelClassError < StandardError; end
 
       require_nested :CloudManager
+      require_nested :InfraManager
+      require_nested :AutomationManager
       require_nested :NetworkManager
       require_nested :StorageManager
-      require_nested :InfraManager
+      require_nested :List
 
       include ::ManagerRefresh::InventoryCollection::Builder::Shared
 
diff --git a/app/models/manager_refresh/inventory_collection/builder/persister_helper.rb b/app/models/manager_refresh/inventory_collection/builder/persister_helper.rb
new file mode 100644
index 00000000000..023b4f4e66b
--- /dev/null
+++ b/app/models/manager_refresh/inventory_collection/builder/persister_helper.rb
@@ -0,0 +1,95 @@
+module ManagerRefresh::InventoryCollection::Builder::PersisterHelper
+  extend ActiveSupport::Concern
+
+  # Interface for creating InventoryCollection under @collections
+  #
+  # @param builder_class    [ManagerRefresh::InventoryCollection::Builder] or subclasses
+  # @param collection_name  [Symbol] used as InventoryCollection:association
+  # @param extra_properties [Hash]   props from InventoryCollection.initialize list
+  #         - adds/overwrites properties added by builder
+  #
+  # @param settings [Hash] builder settings
+  #         - @see ManagerRefresh::InventoryCollection::Builder.default_options
+  #         - @see make_builder_settings()
+  #
+  # @example
+  #   add_collection(ManagerRefresh::InventoryCollection::Builder::CloudManager, :vms) do |builder|
+  #     builder.add_properties(
+  #       :strategy => :local_db_cache_all,
+  #     )
+  #   )
+  #
+  # @see ManagerRefresh::InventoryCollection::Builder
+  #
+  def add_collection(builder_class, collection_name, extra_properties = {}, settings = {}, &block)
+    builder = builder_class.prepare_data(collection_name,
+                                         self.class,
+                                         make_builder_settings(settings),
+                                         &block)
+
+    builder.add_properties(extra_properties) if extra_properties.present?
+
+    builder.add_properties({:manager_uuids => references(collection_name)}, :if_missing) if targeted?
+
+    builder.add_parent_if_missing(manager, targeted?) if manager.present?
+
+    builder.evaluate_lambdas!(self)
+
+    collections[collection_name] = builder.to_inventory_collection
+  end
+
+  def cloud
+    ::ManagerRefresh::InventoryCollection::Builder::CloudManager
+  end
+
+  def network
+    ::ManagerRefresh::InventoryCollection::Builder::NetworkManager
+  end
+
+  def infra
+    ::ManagerRefresh::InventoryCollection::Builder::InfraManager
+  end
+
+  def storage
+    ::ManagerRefresh::InventoryCollection::Builder::StorageManager
+  end
+
+  def automation
+    ::ManagerRefresh::InventoryCollection::Builder::AutomationManager
+  end
+
+  # @param extra_settings [Hash]
+  def make_builder_settings(extra_settings = {})
+    opts = ::ManagerRefresh::InventoryCollection::Builder.default_options
+
+    opts[:adv_settings] = options[:inventory_collections].try(:to_hash) || {}
+    opts[:shared_properties] = shared_options
+
+    opts.merge(extra_settings)
+  end
+
+  def strategy
+    nil
+  end
+
+  # Persisters for targeted refresh can override to true
+  def targeted?
+    false
+  end
+
+  # @return [Hash] kwargs shared for all InventoryCollection objects
+  def shared_options
+    # can be implemented in a subclass
+    {}
+  end
+
+  # TODO: comments - ladas? suggest comment pls :)
+  def references(collection)
+    target.manager_refs_by_association.try(:[], collection).try(:[], :ems_ref).try(:to_a) || []
+  end
+
+  # TODO: comments - ladas? suggest comment pls :)
+  def name_references(collection)
+    target.manager_refs_by_association.try(:[], collection).try(:[], :name).try(:to_a) || []
+  end
+end

From cfae9dca22df4992aa42363206f3fa5db135cc0a Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Thu, 31 May 2018 15:40:35 +0200
Subject: [PATCH 12/18] Added CloudTenant IC definition

---
 .../inventory_collection/builder/network_manager.rb           | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/app/models/manager_refresh/inventory_collection/builder/network_manager.rb b/app/models/manager_refresh/inventory_collection/builder/network_manager.rb
index 3106935a35f..6040092f5a4 100644
--- a/app/models/manager_refresh/inventory_collection/builder/network_manager.rb
+++ b/app/models/manager_refresh/inventory_collection/builder/network_manager.rb
@@ -41,6 +41,10 @@ def floating_ips
           shared_builder_params
         end
 
+        def cloud_tenants
+          shared_builder_params
+        end
+
         def cloud_subnets
           shared_builder_params
         end

From 505afefae54577d765d24363991f953d749f5f18 Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Fri, 1 Jun 2018 10:21:25 +0200
Subject: [PATCH 13/18] Removed adv_settings_enabled option

Flag was for future-use only
---
 app/models/manager_refresh/inventory_collection/builder.rb  | 5 +----
 .../inventory_collection/builder/persister_helper.rb        | 6 ++++--
 .../manager_refresh/inventory_collection/builder_spec.rb    | 6 ------
 3 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/app/models/manager_refresh/inventory_collection/builder.rb b/app/models/manager_refresh/inventory_collection/builder.rb
index 3cc8de00239..76f3d0e951f 100644
--- a/app/models/manager_refresh/inventory_collection/builder.rb
+++ b/app/models/manager_refresh/inventory_collection/builder.rb
@@ -17,8 +17,6 @@ class MissingModelClassError < StandardError; end
       #   :adv_settings
       #     - values from Advanced settings (doesn't overwrite values specified in code)
       #     - @see method ManagerRefresh::Inventory::Persister.options()
-      #   :adv_settings_enabled
-      #     - enable/disable this properties
       #   :auto_missing_parent
       #     - auto assigns parent if this property is missing
       #     - @see method add_parent_if_missing()
@@ -35,7 +33,6 @@ class MissingModelClassError < StandardError; end
       def self.default_options
         {
           :adv_settings              => {},
-          :adv_settings_enabled      => true,
           :auto_missing_parent       => true,
           :auto_model_class          => true,
           :auto_inventory_attributes => true,
@@ -82,7 +79,7 @@ def construct_data
         add_properties(:association => @name)
         add_properties(:model_class => auto_model_class) if @options[:auto_model_class]
 
-        add_properties(@adv_settings, :if_missing) if @options[:adv_settings_enabled]
+        add_properties(@adv_settings, :if_missing)
         add_properties(@shared_properties, :if_missing)
 
         send(@name.to_sym) if respond_to?(@name.to_sym)
diff --git a/app/models/manager_refresh/inventory_collection/builder/persister_helper.rb b/app/models/manager_refresh/inventory_collection/builder/persister_helper.rb
index 023b4f4e66b..12e302d5514 100644
--- a/app/models/manager_refresh/inventory_collection/builder/persister_helper.rb
+++ b/app/models/manager_refresh/inventory_collection/builder/persister_helper.rb
@@ -83,12 +83,14 @@ def shared_options
     {}
   end
 
-  # TODO: comments - ladas? suggest comment pls :)
+  # Returns list of target's ems_refs
+  # @return [Array<String>]
   def references(collection)
     target.manager_refs_by_association.try(:[], collection).try(:[], :ems_ref).try(:to_a) || []
   end
 
-  # TODO: comments - ladas? suggest comment pls :)
+  # Returns list of target's name
+  # @return [Array<String>]
   def name_references(collection)
     target.manager_refs_by_association.try(:[], collection).try(:[], :name).try(:to_a) || []
   end
diff --git a/spec/models/manager_refresh/inventory_collection/builder_spec.rb b/spec/models/manager_refresh/inventory_collection/builder_spec.rb
index a1c90439933..9bbf3bd88b0 100644
--- a/spec/models/manager_refresh/inventory_collection/builder_spec.rb
+++ b/spec/models/manager_refresh/inventory_collection/builder_spec.rb
@@ -63,12 +63,6 @@ def create_persister
 
   # --- adv. settings (TODO: link to gui)---
 
-  it 'works without Advanced settings' do
-    builder = cloud.prepare_data(:vms, persister_class, :adv_settings_enabled => false)
-
-    expect { builder.to_inventory_collection }.not_to raise_error
-  end
-
   it 'assigns Advanced settings' do
     builder = cloud.prepare_data(:tmp, persister_class, :adv_settings => adv_settings)
     data = builder.to_hash

From d8df6d5e16869718012620c74275eb68838bca3c Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Fri, 1 Jun 2018 11:21:19 +0200
Subject: [PATCH 14/18] Removed automatic parent assignment

Should be specified in each persister instead
---
 .../inventory_collection/builder.rb           | 18 -------
 .../builder/persister_helper.rb               |  2 -
 .../inventory_collection/builder_spec.rb      | 51 -------------------
 3 files changed, 71 deletions(-)

diff --git a/app/models/manager_refresh/inventory_collection/builder.rb b/app/models/manager_refresh/inventory_collection/builder.rb
index 76f3d0e951f..f78129ee291 100644
--- a/app/models/manager_refresh/inventory_collection/builder.rb
+++ b/app/models/manager_refresh/inventory_collection/builder.rb
@@ -17,9 +17,6 @@ class MissingModelClassError < StandardError; end
       #   :adv_settings
       #     - values from Advanced settings (doesn't overwrite values specified in code)
       #     - @see method ManagerRefresh::Inventory::Persister.options()
-      #   :auto_missing_parent
-      #     - auto assigns parent if this property is missing
-      #     - @see method add_parent_if_missing()
       #   :auto_model_class
       #     - tries to set model_class from persister class
       #     - @see method auto_model_class
@@ -33,7 +30,6 @@ class MissingModelClassError < StandardError; end
       def self.default_options
         {
           :adv_settings              => {},
-          :auto_missing_parent       => true,
           :auto_model_class          => true,
           :auto_inventory_attributes => true,
           :shared_properties         => {},
@@ -164,20 +160,6 @@ def to_hash
         )
       end
 
-      # Adds parent to InventoryCollection if not set before
-      # Can be disabled by :auto_missing_parent => false
-      def add_parent_if_missing(manager, targeted = false)
-        return unless @options[:auto_missing_parent]
-
-        if manager.present?
-          if targeted && network_manager_collections? && manager.respond_to?(:network_manager)
-            add_properties({:parent => manager.network_manager}, :if_missing)
-          else
-            add_properties({:parent => manager}, :if_missing)
-          end
-        end
-      end
-
       protected
 
       # Extends source hash with
diff --git a/app/models/manager_refresh/inventory_collection/builder/persister_helper.rb b/app/models/manager_refresh/inventory_collection/builder/persister_helper.rb
index 12e302d5514..8026ed01709 100644
--- a/app/models/manager_refresh/inventory_collection/builder/persister_helper.rb
+++ b/app/models/manager_refresh/inventory_collection/builder/persister_helper.rb
@@ -31,8 +31,6 @@ def add_collection(builder_class, collection_name, extra_properties = {}, settin
 
     builder.add_properties({:manager_uuids => references(collection_name)}, :if_missing) if targeted?
 
-    builder.add_parent_if_missing(manager, targeted?) if manager.present?
-
     builder.evaluate_lambdas!(self)
 
     collections[collection_name] = builder.to_inventory_collection
diff --git a/spec/models/manager_refresh/inventory_collection/builder_spec.rb b/spec/models/manager_refresh/inventory_collection/builder_spec.rb
index 9bbf3bd88b0..71a1230acd6 100644
--- a/spec/models/manager_refresh/inventory_collection/builder_spec.rb
+++ b/spec/models/manager_refresh/inventory_collection/builder_spec.rb
@@ -198,55 +198,4 @@ def create_persister
 
     expect(data[:inventory_object_attributes]).to be_empty
   end
-
-  # --- parent ---
-
-  it 'assigns parent to ems automatically if not set' do
-    builder = cloud.prepare_data(:tmp, persister_class)
-    builder.add_parent_if_missing(@ems)
-    data = builder.to_hash
-
-    expect(data[:parent].id).to eq(@ems.id)
-  end
-
-  it 'does not auto-assign parent if set' do
-    builder = cloud.prepare_data(:tmp, persister_class) do |bld|
-      bld.add_properties(:parent => 'some_parent')
-    end
-    builder.add_parent_if_missing(@ems)
-    data = builder.to_hash
-
-    expect(data[:parent]).to eq('some_parent')
-  end
-
-  it 'does not auto-assign parent if switched off' do
-    builder = cloud.prepare_data(:tmp, persister_class, :auto_missing_parent => false)
-    builder.add_parent_if_missing(@ems)
-
-    data = builder.to_hash
-
-    expect(data[:parent]).to be_nil
-  end
-
-  context 'creating network inv. collections' do
-    it "assigns EMS's network manager as parent when targeted refresh" do
-      builder = network.prepare_data(:tmp, persister_class)
-      builder.add_parent_if_missing(@ems, true)
-      data = builder.to_hash
-
-      expect(data[:parent].id).to eq(@ems.network_manager.id)
-    end
-
-    it "assigns EMS instead of its network manager as parent if EMS does not respond to network_manager" do
-      ems_network = FactoryGirl.create(:ems_nuage_network,
-                                       :zone => @zone)
-      expect(ems_network.respond_to?(:network_manager)).to be_falsy
-
-      builder = network.prepare_data(:tmp, persister_class)
-      builder.add_parent_if_missing(ems_network)
-      data = builder.to_hash
-
-      expect(data[:parent].id).to eq(ems_network.id)
-    end
-  end
 end

From 066b406141f565d20e870cff3d76f185cc73ec6c Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Mon, 4 Jun 2018 14:42:38 +0200
Subject: [PATCH 15/18] Fixed require_nested description

Persister's mixin wasn't specified
---
 app/models/manager_refresh/inventory_collection/builder.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/manager_refresh/inventory_collection/builder.rb b/app/models/manager_refresh/inventory_collection/builder.rb
index f78129ee291..b78f38468d4 100644
--- a/app/models/manager_refresh/inventory_collection/builder.rb
+++ b/app/models/manager_refresh/inventory_collection/builder.rb
@@ -9,7 +9,7 @@ class MissingModelClassError < StandardError; end
       require_nested :AutomationManager
       require_nested :NetworkManager
       require_nested :StorageManager
-      require_nested :List
+      require_nested :PersisterHelper
 
       include ::ManagerRefresh::InventoryCollection::Builder::Shared
 

From 365c0fb0f9de35672ff7f59792604cd8118213f7 Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Wed, 6 Jun 2018 11:16:32 +0200
Subject: [PATCH 16/18] Removed IC default auto-settings from builder.

Moved to persister.
---
 .../inventory_collection/builder.rb           | 22 ++++++-------------
 .../builder/persister_helper.rb               |  9 ++++++++
 .../inventory_collection/builder_spec.rb      |  4 ++--
 3 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/app/models/manager_refresh/inventory_collection/builder.rb b/app/models/manager_refresh/inventory_collection/builder.rb
index b78f38468d4..41deb143e7c 100644
--- a/app/models/manager_refresh/inventory_collection/builder.rb
+++ b/app/models/manager_refresh/inventory_collection/builder.rb
@@ -16,24 +16,13 @@ class MissingModelClassError < StandardError; end
       # Default options for builder
       #   :adv_settings
       #     - values from Advanced settings (doesn't overwrite values specified in code)
-      #     - @see method ManagerRefresh::Inventory::Persister.options()
-      #   :auto_model_class
-      #     - tries to set model_class from persister class
-      #     - @see method auto_model_class
-      #   :auto_inventory_attributes
-      #     - auto creates inventory_object_attributes from target model_class setters
-      #     - attributes used in InventoryObject.add_attributes
+      #     - @see method ManagerRefresh::Inventory::Persister.make_builder_settings()
       #   :shared_properties
       #     - any properties applied if missing (not explicitly specified)
-      #   :without_model_class
-      #     - if false and no model_class derived or specified, throws exception
       def self.default_options
         {
-          :adv_settings              => {},
-          :auto_model_class          => true,
-          :auto_inventory_attributes => true,
-          :shared_properties         => {},
-          :without_model_class       => false,
+          :adv_settings      => {},
+          :shared_properties => {},
         }
       end
 
@@ -64,6 +53,9 @@ def initialize(name, persister_class, options = self.class.default_options)
         @dependency_attributes = {}
 
         @options = options
+        @options[:auto_inventory_attributes] = true if @options[:auto_inventory_attributes].nil?
+        @options[:without_model_class] = false if @options[:without_model_class].nil?
+
         @adv_settings = options[:adv_settings] # Configuration/Advanced settings in GUI
         @shared_properties = options[:shared_properties] # From persister
       end
@@ -73,7 +65,7 @@ def initialize(name, persister_class, options = self.class.default_options)
       # Yields for overwriting provider-specific properties
       def construct_data
         add_properties(:association => @name)
-        add_properties(:model_class => auto_model_class) if @options[:auto_model_class]
+        add_properties(:model_class => auto_model_class) unless @options[:without_model_class]
 
         add_properties(@adv_settings, :if_missing)
         add_properties(@shared_properties, :if_missing)
diff --git a/app/models/manager_refresh/inventory_collection/builder/persister_helper.rb b/app/models/manager_refresh/inventory_collection/builder/persister_helper.rb
index 8026ed01709..9b274b05035 100644
--- a/app/models/manager_refresh/inventory_collection/builder/persister_helper.rb
+++ b/app/models/manager_refresh/inventory_collection/builder/persister_helper.rb
@@ -57,11 +57,20 @@ def automation
   end
 
   # @param extra_settings [Hash]
+  #   :auto_inventory_attributes
+  #     - auto creates inventory_object_attributes from target model_class setters
+  #     - attributes used in InventoryObject.add_attributes
+  #   :without_model_class
+  #     - if false and no model_class derived or specified, throws exception
+  #     - doesn't try to derive model class automatically
+  #     - @see method ManagerRefresh::InventoryCollection::Builder.auto_model_class
   def make_builder_settings(extra_settings = {})
     opts = ::ManagerRefresh::InventoryCollection::Builder.default_options
 
     opts[:adv_settings] = options[:inventory_collections].try(:to_hash) || {}
     opts[:shared_properties] = shared_options
+    opts[:auto_inventory_attributes] = true
+    opts[:without_model_class] = false
 
     opts.merge(extra_settings)
   end
diff --git a/spec/models/manager_refresh/inventory_collection/builder_spec.rb b/spec/models/manager_refresh/inventory_collection/builder_spec.rb
index 71a1230acd6..96f3802e9bc 100644
--- a/spec/models/manager_refresh/inventory_collection/builder_spec.rb
+++ b/spec/models/manager_refresh/inventory_collection/builder_spec.rb
@@ -50,13 +50,13 @@ def create_persister
   end
 
   it "doesn't try to derive model_class when disabled" do
-    data = cloud.prepare_data(:vms, persister_class, :auto_model_class => false).to_hash
+    data = cloud.prepare_data(:vms, persister_class, :without_model_class => true).to_hash
 
     expect(data[:model_class]).to be_nil
   end
 
   it 'throws exception if model_class not specified' do
-    builder = cloud.prepare_data(:vms, persister_class, :auto_model_class => false)
+    builder = cloud.prepare_data(:non_existing_ic, persister_class)
 
     expect { builder.to_inventory_collection }.to raise_error(::ManagerRefresh::InventoryCollection::Builder::MissingModelClassError)
   end

From 602fa0ae9ebf39ce70d18604d154ce8536543b10 Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Wed, 6 Jun 2018 12:48:26 +0200
Subject: [PATCH 17/18] Added default IC parent property

---
 app/models/manager_refresh/inventory_collection/builder.rb  | 6 +++++-
 .../inventory_collection/builder/network_manager.rb         | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/app/models/manager_refresh/inventory_collection/builder.rb b/app/models/manager_refresh/inventory_collection/builder.rb
index 41deb143e7c..f5cc34909d6 100644
--- a/app/models/manager_refresh/inventory_collection/builder.rb
+++ b/app/models/manager_refresh/inventory_collection/builder.rb
@@ -235,8 +235,12 @@ def evaluate_builder_params_lambdas!(persister)
         end
       end
 
-      # InventoryCollection definitions for NetworkManager?
       def network_manager_collections?
+        self.class.network_manager_collections?
+      end
+
+      # InventoryCollection definitions for NetworkManager?
+      def self.network_manager_collections?
         false
       end
     end
diff --git a/app/models/manager_refresh/inventory_collection/builder/network_manager.rb b/app/models/manager_refresh/inventory_collection/builder/network_manager.rb
index 6040092f5a4..2a14baccb5c 100644
--- a/app/models/manager_refresh/inventory_collection/builder/network_manager.rb
+++ b/app/models/manager_refresh/inventory_collection/builder/network_manager.rb
@@ -208,7 +208,7 @@ def default_ems_id
         end
 
         # InventoryCollection definitions for NetworkManager?
-        def network_manager_collections?
+        def self.network_manager_collections?
           true
         end
       end

From ed89c0808e635a3d05537c849468a089a131d7b1 Mon Sep 17 00:00:00 2001
From: Martin Slemr <mslemr@redhat.com>
Date: Wed, 6 Jun 2018 14:01:37 +0200
Subject: [PATCH 18/18] Comments for persister helper

---
 .../inventory_collection/builder/persister_helper.rb         | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/app/models/manager_refresh/inventory_collection/builder/persister_helper.rb b/app/models/manager_refresh/inventory_collection/builder/persister_helper.rb
index 9b274b05035..0cc6eed4f18 100644
--- a/app/models/manager_refresh/inventory_collection/builder/persister_helper.rb
+++ b/app/models/manager_refresh/inventory_collection/builder/persister_helper.rb
@@ -36,22 +36,27 @@ def add_collection(builder_class, collection_name, extra_properties = {}, settin
     collections[collection_name] = builder.to_inventory_collection
   end
 
+  # builder_class for add_collection()
   def cloud
     ::ManagerRefresh::InventoryCollection::Builder::CloudManager
   end
 
+  # builder_class for add_collection()
   def network
     ::ManagerRefresh::InventoryCollection::Builder::NetworkManager
   end
 
+  # builder_class for add_collection()
   def infra
     ::ManagerRefresh::InventoryCollection::Builder::InfraManager
   end
 
+  # builder_class for add_collection()
   def storage
     ::ManagerRefresh::InventoryCollection::Builder::StorageManager
   end
 
+  # builder_class for add_collection()
   def automation
     ::ManagerRefresh::InventoryCollection::Builder::AutomationManager
   end