Merge pull request #16981 from Ladas/get_targeted_arel_query_automati…

…cally

Get targeted arel query automatically

app/models/manager_refresh/application_record_iterator.rb

@@ -1,20 +1,25 @@
 module ManagerRefresh
   class ApplicationRecordIterator
-    attr_reader :inventory_collection, :manager_uuids_set, :iterator
+    attr_reader :inventory_collection, :manager_uuids_set, :iterator, :query
 
     # An iterator that can fetch batches of the AR objects based on a set of manager refs, or just mimics AR relation
     # when given an iterator
-    def initialize(inventory_collection: nil, manager_uuids_set: nil, iterator: nil)
+    def initialize(inventory_collection: nil, manager_uuids_set: nil, iterator: nil, query: nil)
       @inventory_collection = inventory_collection
       @manager_uuids_set    = manager_uuids_set
       @iterator             = iterator
+      @query                = query
     end
 
     def find_in_batches(batch_size: 1000)
       if iterator
         iterator.call do |batch|
           yield(batch)
         end
+      elsif query
+        manager_uuids_set.each_slice(batch_size) do |batch|
+          yield(query.where(inventory_collection.targeted_selection_for(batch)))
+        end
       else
         manager_uuids_set.each_slice(batch_size) do |batch|
           yield(inventory_collection.db_collection_for_comparison_for(batch))

app/models/manager_refresh/inventory_collection.rb

@@ -75,7 +75,7 @@ class InventoryCollection
     attr_reader :model_class, :strategy, :attributes_blacklist, :attributes_whitelist, :custom_save_block, :parent,
                 :internal_attributes, :delete_method, :dependency_attributes, :manager_ref, :create_only,
                 :association, :complete, :update_only, :transitive_dependency_attributes, :check_changed, :arel,
-                :inventory_object_attributes, :name, :saver_strategy, :manager_uuids, :builder_params,
+                :inventory_object_attributes, :name, :saver_strategy, :targeted_scope, :builder_params,
                 :targeted_arel, :targeted, :manager_ref_allowed_nil, :use_ar_object,
                 :created_records, :updated_records, :deleted_records,
                 :custom_reconnect_block, :batch_extra_attributes, :references_storage
@@ -324,10 +324,10 @@ class InventoryCollection
     #        ManagerRefresh::InventoryCollection objects, that serve as parents to this InventoryCollection. Then this
     #        InventoryCollection completeness will be encapsulated by the parent_inventory_collections :manager_uuids
     #        instead of this InventoryCollection :manager_uuids.
-    # @param manager_uuids [Array] Array of manager_uuids of the InventoryObjects we want to create/update/delete. Using
+    # @param manager_uuids [Array|Proc] Array of manager_uuids of the InventoryObjects we want to create/update/delete. Using
     #        this attribute, the db_collection_for_comparison will be automatically limited by the manager_uuids, in a
     #        case of a simple relation. In a case of a complex relation, we can leverage :manager_uuids in a
-    #        custom :targeted_arel.
+    #        custom :targeted_arel. We can pass also lambda, for lazy_evaluation.
     # @param all_manager_uuids [Array] Array of all manager_uuids of the InventoryObjects. With the :targeted true,
     #        having this parameter defined will invoke only :delete_method on a complement of this set, making sure
     #        the DB has only this set of data after. This :attribute serves for deleting of top level
@@ -399,7 +399,13 @@ def initialize(model_class: nil, manager_ref: nil, association: nil, parent: nil
       @manager_ref_allowed_nil = manager_ref_allowed_nil || []
 
       # Targeted mode related attributes
-      @manager_uuids                = Set.new.merge(manager_uuids)
+      # TODO(lsmola) Get rid of string references and enforce ManagerRefresh::InventoryCollection::Reference object here
+      @targeted_scope = manager_uuids.each_with_object({}) do |reference, obj|
+        reference_key      = reference.respond_to?(:stringified_reference) ? reference.stringified_reference : reference
+        reference_value    = reference.respond_to?(:stringified_reference) ? reference : nil
+        obj[reference_key] = reference_value
+      end
+      # TODO(lsmola) Should we refactor this to use references too?
       @all_manager_uuids            = all_manager_uuids
       @parent_inventory_collections = parent_inventory_collections
       @targeted_arel                = targeted_arel
@@ -527,12 +533,12 @@ def object_index_with_keys(keys, record)
     def noop?
       # If this InventoryCollection doesn't do anything. it can easily happen for targeted/batched strategies.
       if targeted?
-        if parent_inventory_collections.nil? && manager_uuids.blank? &&
+        if parent_inventory_collections.nil? && targeted_scope.blank? &&
            all_manager_uuids.nil? && parent_inventory_collections.blank? && custom_save_block.nil? &&
            skeletal_primary_index.blank?
           # It's a noop Parent targeted InventoryCollection
           true
-        elsif !parent_inventory_collections.nil? && parent_inventory_collections.all? { |x| x.manager_uuids.blank? } &&
+        elsif !parent_inventory_collections.nil? && parent_inventory_collections.all? { |x| x.targeted_scope.blank? } &&
               skeletal_primary_index.blank?
           # It's a noop Child targeted InventoryCollection
           true
@@ -733,40 +739,78 @@ def batch_size_pure_sql
       10_000
     end
 
+    def manager_uuids
+      # TODO(lsmola) LEGACY: this is still being used by :targetel_arel definitions and it expects array of strings
+      raise "This works only for :manager_ref size 1" if manager_ref.size > 1
+      key = manager_ref.first
+      transform_references_to_hashes(targeted_scope).map { |x| x[key] }
+    end
+
     def build_multi_selection_condition(hashes, keys = nil)
       keys ||= manager_ref
-      table_name = model_class.table_name
-      cond_data  = hashes.map do |hash|
-        "(#{keys.map { |x| ActiveRecord::Base.connection.quote(hash[x]) }.join(",")})"
-      end.join(",")
-      column_names = keys.map { |key| "#{table_name}.#{ActiveRecord::Base.connection.quote_column_name(key)}" }.join(",")
 
-      "(#{column_names}) IN (#{cond_data})"
+      multi_selection_and_or_query(keys, hashes)
+    end
+
+    def multi_selection_and_or_query(keys, hashes)
+      arel_table = model_class.arel_table
+      # We do pure SQL OR, since Arel is nesting every .or into another parentheses, otherwise this would be just
+      # inject(:or) instead of to_sql with .join(" OR ")
+      hashes.map { |hash| "(#{keys.map { |key| arel_table[key].eq(hash[key]) }.inject(:and).to_sql})" }.join(" OR ")
     end
 
     def db_collection_for_comparison
       if targeted?
         if targeted_arel.respond_to?(:call)
           targeted_arel.call(self)
-        elsif manager_ref.count > 1
-          # TODO(lsmola) optimize with ApplicationRecordIterator
-          hashes = extract_references(manager_uuids)
-          full_collection_for_comparison.where(build_multi_selection_condition(hashes))
+        elsif parent_inventory_collections.present?
+          targeted_arel_default
         else
-          ManagerRefresh::ApplicationRecordIterator.new(
-            :inventory_collection => self,
-            :manager_uuids_set    => manager_uuids.to_a.flatten.compact
-          )
+          targeted_iterator_for(targeted_scope)
         end
       else
         full_collection_for_comparison
       end
     end
 
+    # Builds targeted query limiting the results by the :references defined in parent_inventory_collections
+    def targeted_arel_default
+      if parent_inventory_collections.collect { |x| x.model_class.base_class }.uniq.count > 1
+        raise "Multiple :parent_inventory_collections with different base class are not supported by default. Write "\
+              ":targeted_arel manually, or separate [#{self}] into 2 InventoryCollection objects."
+      end
+      parent_collection = parent_inventory_collections.first
+      references        = parent_inventory_collections.collect(&:targeted_scope).reduce({}, :merge!)
+
+      parent_collection.targeted_iterator_for(references, full_collection_for_comparison)
+    end
+
+    def transform_references_to_hashes(references)
+      # TODO(lsmola) remove when we ensure only ManagerRefresh::InventoryCollection::Reference is in targeted_scope
+      string_references, references = references.partition { |_key, value| value.nil? }
+
+      hash_references = references.map { |x| x.second.full_reference }
+      hash_references.concat(extract_references(string_references.map(&:first)))
+    end
+
+    def targeted_selection_for(references)
+      build_multi_selection_condition(transform_references_to_hashes(references))
+    end
+
+    def targeted_iterator_for(references, query = nil)
+      ManagerRefresh::ApplicationRecordIterator.new(
+        :inventory_collection => self,
+        :manager_uuids_set    => references,
+        :query                => query
+      )
+    end
+
     # Extracting references to a relation friendly format
     #
     # @param new_references [Array] array of index_values of the InventoryObjects
     def extract_references(new_references = [])
+      # TODO(lsmola) Remove this when we allow only ManagerRefresh::InventoryCollection::Reference, decoding/encoding
+      # from string is ugly
       hash_uuids_by_ref = []
 
       new_references.each do |index_value|
@@ -775,17 +819,16 @@ def extract_references(new_references = [])
         uuids = index_value.split("__")
 
         reference = {}
-        attribute_names.each_with_index do |ref, uuid_value|
+        manager_ref.each_with_index do |ref, uuid_value|
           reference[ref] = uuids[uuid_value]
         end
         hash_uuids_by_ref << reference
       end
       hash_uuids_by_ref
     end
 
-    def db_collection_for_comparison_for(manager_uuids_set)
-      # TODO(lsmola) this should have the build_multi_selection_condition, like in the method above
-      full_collection_for_comparison.where(manager_ref.first => manager_uuids_set)
+    def db_collection_for_comparison_for(references)
+      full_collection_for_comparison.where(targeted_selection_for(references))
     end
 
     def db_collection_for_comparison_for_complement_of(manager_uuids_set)

app/models/manager_refresh/inventory_collection/scanner.rb

@@ -41,7 +41,7 @@ def scan!(inventory_collections)
       delegate :attribute_references,
                :data_collection_finalized=,
                :dependency_attributes,
-               :manager_uuids,
+               :targeted_scope,
                :parent_inventory_collections,
                :parent_inventory_collections=,
                :references,
@@ -60,9 +60,7 @@ def scan!
 
           if targeted? && parent_inventory_collections.blank?
             # We want to track what manager_uuids we should query from a db, for the targeted refresh
-            # TODO(lsmola) this has to track references
-            manager_uuid = inventory_object.manager_uuid
-            manager_uuids << manager_uuid if manager_uuid
+            targeted_scope[inventory_object.manager_uuid] = inventory_object.reference
           end
         end
 

app/models/manager_refresh/inventory_collection_default.rb

@@ -114,13 +114,6 @@ def hardwares(extra_attributes = {})
         :use_ar_object                => true,
       }
 
-      attributes[:targeted_arel] = lambda do |inventory_collection|
-        manager_uuids = inventory_collection.parent_inventory_collections.flat_map { |c| c.manager_uuids.to_a }
-        inventory_collection.parent.hardwares.joins(:vm_or_template).where(
-          'vms' => {:ems_ref => manager_uuids}
-        )
-      end
-
       attributes.merge!(extra_attributes)
     end
 
@@ -139,13 +132,6 @@ def operating_systems(extra_attributes = {})
         ],
       }
 
-      attributes[:targeted_arel] = lambda do |inventory_collection|
-        manager_uuids = inventory_collection.parent_inventory_collections.flat_map { |c| c.manager_uuids.to_a }
-        inventory_collection.parent.operating_systems.joins(:vm_or_template).where(
-          'vms' => {:ems_ref => manager_uuids}
-        )
-      end
-
       attributes.merge!(extra_attributes)
     end
 
@@ -171,13 +157,6 @@ def disks(extra_attributes = {})
         ],
       }
 
-      attributes[:targeted_arel] = lambda do |inventory_collection|
-        manager_uuids = inventory_collection.parent_inventory_collections.flat_map { |c| c.manager_uuids.to_a }
-        inventory_collection.parent.disks.joins(:hardware => :vm_or_template).where(
-          :hardware => {'vms' => {:ems_ref => manager_uuids}}
-        )
-      end
-
       attributes.merge!(extra_attributes)
     end
   end

app/models/manager_refresh/inventory_collection_default/cloud_manager.rb

@@ -54,13 +54,6 @@ def vm_and_template_labels(extra_attributes = {})
         ]
       }
 
-      attributes[:targeted_arel] = lambda do |inventory_collection|
-        manager_uuids = inventory_collection.parent_inventory_collections.collect(&:manager_uuids).map(&:to_a).flatten
-        inventory_collection.parent.vm_and_template_labels.where(
-          'vms' => {:ems_ref => manager_uuids}
-        )
-      end
-
       attributes.merge!(extra_attributes)
     end
 
@@ -72,13 +65,6 @@ def networks(extra_attributes = {})
         :parent_inventory_collections => [:vms],
       }
 
-      attributes[:targeted_arel] = lambda do |inventory_collection|
-        manager_uuids = inventory_collection.parent_inventory_collections.flat_map { |c| c.manager_uuids.to_a }
-        inventory_collection.parent.networks.joins(:hardware => :vm_or_template).where(
-          :hardware => {'vms' => {:ems_ref => manager_uuids}}
-        )
-      end
-
       attributes.merge!(extra_attributes)
     end
 
@@ -102,13 +88,6 @@ def orchestration_stacks_resources(extra_attributes = {})
         :parent_inventory_collections => [:orchestration_stacks]
       }
 
-      extra_attributes[:targeted_arel] = lambda do |inventory_collection|
-        manager_uuids = inventory_collection.parent_inventory_collections.flat_map { |c| c.manager_uuids.to_a }
-        inventory_collection.parent.orchestration_stacks_resources.references(:orchestration_stacks).where(
-          :orchestration_stacks => {:ems_ref => manager_uuids}
-        )
-      end
-
       attributes.merge!(extra_attributes)
     end
 
@@ -119,13 +98,6 @@ def orchestration_stacks_outputs(extra_attributes = {})
         :parent_inventory_collections => [:orchestration_stacks],
       }
 
-      extra_attributes[:targeted_arel] = lambda do |inventory_collection|
-        manager_uuids = inventory_collection.parent_inventory_collections.flat_map { |c| c.manager_uuids.to_a }
-        inventory_collection.parent.orchestration_stacks_outputs.references(:orchestration_stacks).where(
-          :orchestration_stacks => {:ems_ref => manager_uuids}
-        )
-      end
-
       attributes.merge!(extra_attributes)
     end
 
@@ -136,13 +108,6 @@ def orchestration_stacks_parameters(extra_attributes = {})
         :parent_inventory_collections => [:orchestration_stacks],
       }
 
-      extra_attributes[:targeted_arel] = lambda do |inventory_collection|
-        manager_uuids = inventory_collection.parent_inventory_collections.flat_map { |c| c.manager_uuids.to_a }
-        inventory_collection.parent.orchestration_stacks_parameters.references(:orchestration_stacks).where(
-          :orchestration_stacks => {:ems_ref => manager_uuids}
-        )
-      end
-
       attributes.merge!(extra_attributes)
     end
 

app/models/manager_refresh/inventory_collection_default/network_manager.rb

@@ -89,13 +89,6 @@ def firewall_rules(extra_attributes = {})
         :parent_inventory_collections => [:security_groups],
       }
 
-      extra_attributes[:targeted_arel] = lambda do |inventory_collection|
-        manager_uuids = inventory_collection.parent_inventory_collections.flat_map { |c| c.manager_uuids.to_a }
-        inventory_collection.parent.firewall_rules.references(:security_groups).where(
-          :security_groups => {:ems_ref => manager_uuids}
-        )
-      end
-
       attributes.merge!(extra_attributes)
     end