Merge pull request #15823 from Ladas/inventory_collection_saver_refac…

…toring

Inventory collection saver refactoring

app/models/manager_refresh/inventory_collection.rb

@@ -341,9 +341,6 @@ class InventoryCollection
     #             inventory_object[:label] = inventory_object[:name]
     #           So by using inventory_object_attributes, we will be guarding the allowed attributes and will have an
     #           explicit list of allowed attributes, that can be used also for documentation purposes.
-    # @param unique_index_columns [Array] Array of symbols identifying columns of a DB unique index, we will be using.
-    #        If there is only 1 unique index, this will be auto-discovered, otherwise we need to specify the correct
-    #        one.
     # @param name [Symbol] A unique name of the InventoryCollection under a Persister. If not provided, the :association
     #        attribute is used. If :association is nil as well, the :name will be inferred from the :model_class.
     # @param saver_strategy [Symbol] A strategy that will be used for InventoryCollection persisting into the DB.
@@ -407,7 +404,7 @@ def initialize(model_class: nil, manager_ref: nil, association: nil, parent: nil
                    custom_save_block: nil, delete_method: nil, dependency_attributes: nil,
                    attributes_blacklist: nil, attributes_whitelist: nil, complete: nil, update_only: nil,
                    check_changed: nil, custom_manager_uuid: nil, custom_db_finder: nil, arel: nil, builder_params: {},
-                   inventory_object_attributes: nil, unique_index_columns: nil, name: nil, saver_strategy: nil,
+                   inventory_object_attributes: nil, name: nil, saver_strategy: nil,
                    parent_inventory_collections: nil, manager_uuids: [], all_manager_uuids: nil, targeted_arel: nil,
                    targeted: nil, manager_ref_allowed_nil: nil, secondary_refs: {}, use_ar_object: nil,
                    custom_reconnect_block: nil, batch_extra_attributes: [])
@@ -430,7 +427,6 @@ def initialize(model_class: nil, manager_ref: nil, association: nil, parent: nil
       @complete               = complete.nil? ? true : complete
       @update_only            = update_only.nil? ? false : update_only
       @builder_params         = builder_params
-      @unique_index_columns   = unique_index_columns
       @name                   = name || association || model_class.to_s.demodulize.tableize
       @saver_strategy         = process_saver_strategy(saver_strategy)
       @use_ar_object          = use_ar_object || false
@@ -628,60 +624,10 @@ def noop?
       end
     end
 
-    def supports_sti?
-      @supports_sti_cache = model_class.column_names.include?("type") if @supports_sti_cache.nil?
-      @supports_sti_cache
-    end
-
-    def supports_created_on?
-      if @supports_created_on_cache.nil?
-        @supports_created_on_cache = (model_class.column_names.include?("created_on") && ActiveRecord::Base.record_timestamps)
-      end
-      @supports_created_on_cache
-    end
-
-    def supports_updated_on?
-      if @supports_updated_on_cache.nil?
-        @supports_updated_on_cache = (model_class.column_names.include?("updated_on") && ActiveRecord::Base.record_timestamps)
-      end
-      @supports_updated_on_cache
-    end
-
-    def supports_created_at?
-      if @supports_created_at_cache.nil?
-        @supports_created_at_cache = (model_class.column_names.include?("created_at") && ActiveRecord::Base.record_timestamps)
-      end
-      @supports_created_at_cache
-    end
-
-    def supports_updated_at?
-      if @supports_updated_at_cache.nil?
-        @supports_updated_at_cache = (model_class.column_names.include?("updated_at") && ActiveRecord::Base.record_timestamps)
-      end
-      @supports_updated_at_cache
-    end
-
     def targeted?
       targeted
     end
 
-    def unique_index_columns
-      return @unique_index_columns if @unique_index_columns
-
-      unique_indexes = model_class.connection.indexes(model_class.table_name).select(&:unique)
-      if unique_indexes.count > 1
-        raise "Cannot infer unique index automatically, since the table #{model_class.table_name}"\
-              " of the #{self} contains more than 1 unique index: '#{unique_indexes.collect(&:name)}'. Please define"\
-              " the unique index columns explicitly as: :unique_index => [:column1, :column2, etc.]"
-      end
-
-      if unique_indexes.blank?
-        raise "#{self} and its table #{model_class.table_name} must have a unique index defined, in order to use "\
-              "saver_strategy :concurrent_safe or :concurrent_safe_batch."
-      end
-      @unique_index_columns = unique_indexes.first.columns.map(&:to_sym)
-    end
-
     def <<(inventory_object)
       unless data_index[inventory_object.manager_uuid]
         data_index[inventory_object.manager_uuid] = inventory_object

app/models/manager_refresh/inventory_collection_default/cloud_manager.rb

@@ -6,7 +6,6 @@ def vms(extra_attributes = {})
         :association            => :vms,
         :delete_method          => :disconnect_inv,
         :attributes_blacklist   => [:genealogy_parent],
-        :unique_index_columns   => [:ems_id, :ems_ref],
         :use_ar_object          => true, # Because of raw_power_state setter
         # TODO(lsmola) can't do batch strategy for vms because of key_pairs relation
         :saver_strategy         => :default,
@@ -27,7 +26,6 @@ def miq_templates(extra_attributes = {})
         :association            => :miq_templates,
         :delete_method          => :disconnect_inv,
         :attributes_blacklist   => [:genealogy_parent],
-        :unique_index_columns   => [:ems_id, :ems_ref],
         :use_ar_object          => true, # Because of raw_power_state setter
         :batch_extra_attributes => [:power_state, :state_changed_on, :previous_state],
         :builder_params         => {

app/models/manager_refresh/save_collection/saver/base.rb

@@ -8,20 +8,20 @@ class Base
 
       def initialize(inventory_collection)
         @inventory_collection = inventory_collection
+        # TODO(lsmola) do I need to reload every time? Also it should be enough to clear the associations.
+        inventory_collection.parent.reload if inventory_collection.parent
+        @association = inventory_collection.db_collection_for_comparison
 
         # Private attrs
-        @primary_key            = inventory_collection.model_class.primary_key
-        @arel_primary_key       = inventory_collection.model_class.arel_attribute(primary_key)
+        @model_class            = inventory_collection.model_class
+        @primary_key            = @model_class.primary_key
+        @arel_primary_key       = @model_class.arel_attribute(@primary_key)
         @unique_index_keys      = inventory_collection.manager_ref_to_cols.map(&:to_sym)
         @unique_index_keys_to_s = inventory_collection.manager_ref_to_cols.map(&:to_s)
         @select_keys            = [@primary_key] + @unique_index_keys_to_s
         @unique_db_primary_keys = Set.new
         @unique_db_indexes      = Set.new
 
-        # TODO(lsmola) do I need to reload every time? Also it should be enough to clear the associations.
-        inventory_collection.parent.reload if inventory_collection.parent
-        @association = inventory_collection.db_collection_for_comparison
-
         # Right now ApplicationRecordIterator in association is used for targeted refresh. Given the small amount of
         # records flowing through there, we probably don't need to optimize that association to fetch a pure SQL.
         @pure_sql_records_fetching = !inventory_collection.use_ar_object? && !@association.kind_of?(ManagerRefresh::ApplicationRecordIterator)
@@ -47,7 +47,7 @@ def save_inventory_collection!
 
       attr_reader :unique_index_keys, :unique_index_keys_to_s, :select_keys, :unique_db_primary_keys, :unique_db_indexes,
                   :primary_key, :arel_primary_key, :record_key_method, :pure_sql_records_fetching, :select_keys_indexes,
-                  :batch_size, :batch_size_for_persisting
+                  :batch_size, :batch_size_for_persisting, :model_class
 
       def save!(association)
         attributes_index        = {}
@@ -191,21 +191,76 @@ def time_now
         end
       end
 
-      def supports_remote_data_timestamp?(all_attribute_keys)
-        all_attribute_keys.include?(:remote_data_timestamp) # include? on Set is O(1)
-      end
-
       def assign_attributes_for_update!(hash, update_time)
-        hash[:updated_on]   = update_time if inventory_collection.supports_updated_on?
-        hash[:updated_at]   = update_time if inventory_collection.supports_updated_at?
+        hash[:updated_on]   = update_time if supports_updated_on?
+        hash[:updated_at]   = update_time if supports_updated_at?
       end
 
       def assign_attributes_for_create!(hash, create_time)
-        hash[:type]         = inventory_collection.model_class.name if inventory_collection.supports_sti? && hash[:type].nil?
-        hash[:created_on]   = create_time if inventory_collection.supports_created_on?
-        hash[:created_at]   = create_time if inventory_collection.supports_created_at?
+        hash[:type]         = model_class.name if supports_sti? && hash[:type].nil?
+        hash[:created_on]   = create_time if supports_created_on?
+        hash[:created_at]   = create_time if supports_created_at?
         assign_attributes_for_update!(hash, create_time)
       end
+
+      def unique_index_columns
+        return @unique_index_columns if @unique_index_columns
+
+        if model_class.respond_to?(:manager_refresh_unique_index_columns)
+          return @unique_index_columns = model_class.manager_refresh_unique_index_columns
+        end
+
+        unique_indexes = model_class.connection.indexes(model_class.table_name).select(&:unique)
+        if unique_indexes.count > 1
+          raise "Cannot infer unique index automatically, since the table #{model_class.table_name}"\
+                " of the #{inventory_collection} contains more than 1 unique index: '#{unique_indexes.collect(&:name)}'."\
+                " Please define the unique index columns explicitly on a model as a class method"\
+                " self.manager_refresh_unique_index_columns returning [:column1, :column2, etc.]"
+        end
+
+        if unique_indexes.blank?
+          raise "#{inventory_collection} and its table #{model_class.table_name} must have a unique index defined, to"\
+                " be able to use saver_strategy :concurrent_safe or :concurrent_safe_batch."
+        end
+        @unique_index_columns = unique_indexes.first.columns.map(&:to_sym)
+      end
+
+      def supports_sti?
+        @supports_sti_cache = model_class.column_names.include?("type") if @supports_sti_cache.nil?
+        @supports_sti_cache
+      end
+
+      def supports_created_on?
+        if @supports_created_on_cache.nil?
+          @supports_created_on_cache = (model_class.column_names.include?("created_on") && ActiveRecord::Base.record_timestamps)
+        end
+        @supports_created_on_cache
+      end
+
+      def supports_updated_on?
+        if @supports_updated_on_cache.nil?
+          @supports_updated_on_cache = (model_class.column_names.include?("updated_on") && ActiveRecord::Base.record_timestamps)
+        end
+        @supports_updated_on_cache
+      end
+
+      def supports_created_at?
+        if @supports_created_at_cache.nil?
+          @supports_created_at_cache = (model_class.column_names.include?("created_at") && ActiveRecord::Base.record_timestamps)
+        end
+        @supports_created_at_cache
+      end
+
+      def supports_updated_at?
+        if @supports_updated_at_cache.nil?
+          @supports_updated_at_cache = (model_class.column_names.include?("updated_at") && ActiveRecord::Base.record_timestamps)
+        end
+        @supports_updated_at_cache
+      end
+
+      def supports_remote_data_timestamp?(all_attribute_keys)
+        all_attribute_keys.include?(:remote_data_timestamp) # include? on Set is O(1)
+      end
     end
   end
 end

app/models/manager_refresh/save_collection/saver/concurrent_safe_batch.rb

@@ -58,10 +58,10 @@ def save!(association)
           all_attribute_keys.merge(attributes_index[index].keys)
         end
 
-        all_attribute_keys << :created_at if inventory_collection.supports_created_at?
-        all_attribute_keys << :updated_at if inventory_collection.supports_updated_at?
-        all_attribute_keys << :created_on if inventory_collection.supports_created_on?
-        all_attribute_keys << :updated_on if inventory_collection.supports_updated_on?
+        all_attribute_keys << :created_at if supports_created_at?
+        all_attribute_keys << :updated_at if supports_updated_at?
+        all_attribute_keys << :created_on if supports_created_on?
+        all_attribute_keys << :updated_on if supports_updated_on?
 
         _log.info("*************** PROCESSING #{inventory_collection} of size #{inventory_collection.size} *************")
 
@@ -73,7 +73,7 @@ def save!(association)
           inventory_collection.custom_reconnect_block.call(inventory_collection, inventory_objects_index, attributes_index)
         end
 
-        all_attribute_keys << :type if inventory_collection.supports_sti?
+        all_attribute_keys << :type if supports_sti?
         # Records that were not found in the DB but sent for saving, we will be creating these in the DB.
         if inventory_collection.create_allowed?
           inventory_objects_index.each_slice(batch_size_for_persisting) do |batch|

app/models/manager_refresh/save_collection/saver/sql_helper.rb

@@ -1,10 +1,6 @@
 module ManagerRefresh::SaveCollection
   module Saver
     module SqlHelper
-      def unique_index_columns
-        inventory_collection.unique_index_columns
-      end
-
       def on_conflict_update
         true
       end

app/models/vm_or_template.rb

@@ -278,6 +278,10 @@ def self.model_suffix
     manager_class.short_token
   end
 
+  def self.manager_refresh_unique_index_columns
+    [:ems_id, :ems_ref]
+  end
+
   def to_s
     name
   end