Targeted scope serialization

app/models/manager_refresh/inventory/persister.rb

@@ -174,7 +174,7 @@ def add_remaining_inventory_collections(defaults, options = {})
   # @return [Hash] entire Persister object serialized to hash
   def to_hash
     collections_data = collections.map do |_, collection|
-      next if collection.data.blank? && collection.targeted_scope.blank? && collection.all_manager_uuids.nil?
+      next if collection.data.blank? && collection.targeted_scope.primary_references.blank? && collection.all_manager_uuids.nil?
 
       collection.to_hash
     end.compact

app/models/manager_refresh/inventory_collection.rb

@@ -432,12 +432,6 @@ def initialize(model_class: nil, manager_ref: nil, association: nil, parent: nil
       @manager_ref_allowed_nil = manager_ref_allowed_nil || []
 
       # Targeted mode related attributes
-      # 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
@@ -454,6 +448,7 @@ def initialize(model_class: nil, manager_ref: nil, association: nil, parent: nil
 
       @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)
 
       @created_records = []
       @updated_records = []
@@ -611,12 +606,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? && targeted_scope.blank? &&
+        if parent_inventory_collections.nil? && targeted_scope.primary_references.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.targeted_scope.blank? } &&
+        elsif !parent_inventory_collections.nil? && parent_inventory_collections.all? { |x| x.targeted_scope.primary_references.blank? } &&
               skeletal_primary_index.blank?
           # It's a noop Child targeted InventoryCollection
           true
@@ -863,7 +858,7 @@ 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] }
+      transform_references_to_hashes(targeted_scope.primary_references).map { |x| x[key] }
     end
 
     # Builds a multiselection conditions like (table1.a = a1 AND table2.b = b1) OR (table1.a = a2 AND table2.b = b2)
@@ -886,7 +881,7 @@ def db_collection_for_comparison
         elsif parent_inventory_collections.present?
           targeted_arel_default
         else
-          targeted_iterator_for(targeted_scope)
+          targeted_iterator_for(targeted_scope.primary_references)
         end
       else
         full_collection_for_comparison
@@ -902,21 +897,22 @@ def targeted_arel_default
               ":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!)
+      references        = parent_inventory_collections.map { |x| x.targeted_scope.primary_references }.reduce({}, :merge!)
 
       parent_collection.targeted_iterator_for(references, full_collection_for_comparison)
     end
 
     # Gets targeted references and transforms them into list of hashes
     #
-    # @param references [Hash{String => ManagerRefresh::InventoryCollection::Reference}] passed references
+    # @param references [Array, ManagerRefresh::Inventorycollection::TargetedScope] passed references
     # @return [Array<Hash>] References transformed into the array of hashes
     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)))
+      if references.kind_of?(Array)
+        # Sliced ManagerRefresh::Inventorycollection::TargetedScope
+        references.map { |x| x.second.full_reference }
+      else
+        references.values.map(&:full_reference)
+      end
     end
 
     # Builds a multiselection conditions like (table1.a = a1 AND table2.b = b1) OR (table1.a = a2 AND table2.b = b2)
@@ -930,7 +926,7 @@ def targeted_selection_for(references)
 
     # Returns iterator for the passed references and a query
     #
-    # @param references [Hash{String => ManagerRefresh::InventoryCollection::Reference}] assed references
+    # @param references [Hash{String => ManagerRefresh::InventoryCollection::Reference}] Passed references
     # @param query [ActiveRecord::Relation] relation that can fetch all data of this InventoryCollection from the DB
     # @return [ManagerRefresh::ApplicationRecordIterator] Iterator for the references and query
     def targeted_iterator_for(references, query = nil)
@@ -941,29 +937,6 @@ def targeted_iterator_for(references, query = nil)
       )
     end
 
-    # Extracting references to a relation friendly format
-    #
-    # @param new_references [Array<String>] array of index_values of the InventoryObjects
-    # @return [Array<Hash>] extracted references
-    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|
-        next if index_value.nil?
-        # TODO(lsmola) no need when hashes are the original hashes
-        uuids = index_value.split("__")
-
-        reference = {}
-        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
-
     # Builds an ActiveRecord::Relation that can fetch all the references from the DB
     #
     # @param references [Hash{String => ManagerRefresh::InventoryCollection::Reference}] passed references

app/models/manager_refresh/inventory_collection/index/proxy.rb

@@ -114,6 +114,10 @@ def named_ref(ref = primary_index_ref)
           all_refs[ref]
         end
 
+        def primary_index_ref
+          :manager_ref
+        end
+
         private
 
         delegate :association_to_foreign_key_mapping,
@@ -146,10 +150,6 @@ def local_db_index_find(reference)
           local_db_index(reference.ref).find(reference)
         end
 
-        def primary_index_ref
-          :manager_ref
-        end
-
         def data_index(name)
           data_indexes[name] || raise("Index :#{name} not defined for #{inventory_collection}")
         end

app/models/manager_refresh/inventory_collection/reference.rb

@@ -74,7 +74,7 @@ def build_full_reference(data, keys)
         if data.kind_of?(Hash)
           data
         else
-          # assert_index makes sure that only keys of size 1 can go here
+          raise "Please provide Hash as a reference, :manager_ref count includes more then 1 attribute. keys: #{keys}, data: #{data}" if keys.size > 1
           {keys.first => data}
         end
       end

app/models/manager_refresh/inventory_collection/references_storage.rb

@@ -10,37 +10,97 @@ class ReferencesStorage
       attr_reader :attribute_references
 
       def initialize(index_proxy)
-        @index_proxy          = index_proxy
-        @references           = {}
-        @attribute_references = Set.new
+        @index_proxy                   = index_proxy
+        @references                    = {}
+        @references[primary_index_ref] = {}
+        @attribute_references          = Set.new
       end
 
-      def add_reference(reference, key: nil)
-        (references[reference.ref] ||= {})[reference.stringified_reference] = reference unless references[reference.stringified_reference]
+      # Adds reference to the storage. The reference can be already existing, otherwise we attempt to build it.
+      #
+      # @param reference_data [ManagerRefresh::InventoryCollection::References, Hash, Object] Either existing Reference
+      #        object, or data we will build the reference object from. For InventoryCollection with :manager_ref size
+      #        bigger than 1, it's required to pass a Hash.
+      # @param key [String] If the reference comes from a InventoryObjectLazy, pointing to specific attribute using :key
+      #        we want to record what attribute was referenced.
+      # @param ref [Symbol] A key to specific reference, if it's a reference pointing to something else than primary
+      #        index.
+      def add_reference(reference_data, key: nil, ref: nil)
+        reference           = build_reference(reference_data, ref)
+        specific_references = references[reference.ref] ||= {}
+
+        specific_references[reference.stringified_reference] = reference
 
         # If we access an attribute of the value, using a :key, we want to keep a track of that
         attribute_references << key if key
       end
 
-      def build_reference(index_data, ref = :manager_ref)
-        return index_data if index_data.kind_of?(::ManagerRefresh::InventoryCollection::Reference)
+      # Adds reference to the storage. The reference can be already existing, otherwise we attempt to build it. This is
+      # simplified version of add_reference, not allowing to define :key or :ref.
+      #
+      # @param reference_data [ManagerRefresh::InventoryCollection::References, Hash, Object] Either existing Reference
+      #        object, or data we will build the reference object from. For InventoryCollection with :manager_ref size
+      #        bigger than 1, it's required to pass a Hash.
+      def <<(reference_data)
+        add_reference(reference_data)
+      end
+
+      # Adds array of references to the storage. The reference can be already existing, otherwise we attempt to build
+      # it.
+      #
+      # @param references_array [Array] Array of reference objects acceptable by add_reference method.
+      # @param ref [Symbol] A key to specific reference, if it's a reference pointing to something else than primary
+      #        index.
+      # @return [ManagerRefresh::InventoryCollection::ReferencesStorage] Returns self
+      def merge!(references_array, ref: nil)
+        references_array.each { |reference_data| add_reference(reference_data, :ref => ref) }
+        self
+      end
+
+      # @return [Hash{String => ManagerRefresh::InventoryCollection::Reference}] Hash of indexed Reference objects
+      def primary_references
+        references[primary_index_ref]
+      end
+
+      # Builds a Reference object
+      #
+      # @param reference_data [ManagerRefresh::InventoryCollection::References, Hash, Object] Either existing Reference
+      #        object, or data we will build the reference object from. For InventoryCollection with :manager_ref size
+      #        bigger than 1, it's required to pass a Hash.
+      def build_reference(reference_data, ref = nil)
+        ref ||= primary_index_ref
+        return reference_data if reference_data.kind_of?(::ManagerRefresh::InventoryCollection::Reference)
 
-        ::ManagerRefresh::InventoryCollection::Reference.new(index_data, ref, named_ref(ref))
+        ::ManagerRefresh::InventoryCollection::Reference.new(reference_data, ref, named_ref(ref))
       end
 
-      def build_stringified_reference(index_data, keys)
-        ::ManagerRefresh::InventoryCollection::Reference.build_stringified_reference(index_data, keys)
+      # Builds string uuid from passed Hash and keys
+      #
+      # @param hash [Hash] Hash data
+      # @param keys [Array<Symbol>] Indexes into the Hash data
+      # @return [String] Concatenated values on keys from data
+      def build_stringified_reference(hash, keys)
+        ::ManagerRefresh::InventoryCollection::Reference.build_stringified_reference(hash, keys)
       end
 
+      # Builds string uuid from passed Object and keys
+      #
+      # @param record [ApplicationRecord] ActiveRecord record
+      # @param keys [Array<Symbol>] Indexes into the Hash data
+      # @return [String] Concatenated values on keys from data
       def build_stringified_reference_for_record(record, keys)
         ::ManagerRefresh::InventoryCollection::Reference.build_stringified_reference_for_record(record, keys)
       end
 
       private
 
+      # @return [ManagerRefresh::InventoryCollection::Index::Proxy] Index::Proxy object associated to this reference
+      #         storage
       attr_reader :index_proxy
 
-      delegate :named_ref, :to => :index_proxy
+      delegate :named_ref,
+               :primary_index_ref,
+               :to => :index_proxy
     end
   end
 end

app/models/manager_refresh/inventory_collection/scanner.rb

@@ -6,7 +6,7 @@ class << self
         # themselves. Dependencies are needed for building a graph, references are needed for effective DB querying, where
         # we can load all referenced objects of some InventoryCollection by one DB query.
         #
-        # @param inventory_collections [Array] Array fo
+        # @param inventory_collections [Array<ManagerRefresh::InventoryCollection>] Array of InventoryCollection objects
         def scan!(inventory_collections)
           indexed_inventory_collections = inventory_collections.index_by(&:name)
 
@@ -60,7 +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
-            targeted_scope[inventory_object.manager_uuid] = inventory_object.reference
+            targeted_scope << inventory_object.reference
           end
         end
 

app/models/manager_refresh/inventory_collection/serialization.rb

@@ -24,12 +24,13 @@ def initialize(inventory_collection)
       # @param available_inventory_collections [Array<ManagerRefresh::InventoryCollection>] List of available
       #        InventoryCollection objects
       def from_hash(inventory_objects_data, available_inventory_collections)
+        targeted_scope.merge!(inventory_objects_data["manager_uuids"].map(&:symbolize_keys!))
+
         inventory_objects_data['data'].each do |inventory_object_data|
           build(hash_to_data(inventory_object_data, available_inventory_collections).symbolize_keys!)
         end
 
-        # TODO(lsmola) we need to remodel the targeted scope, to be able to serialize targeted InventoryCollections
-        # self.targeted_scope.merge!(inventory_objects_data['manager_uuids'] || [])
+        # TODO(lsmola) add support for all_manager_uuids serialization
         # self.all_manager_uuids = inventory_objects_data['all_manager_uuids']
       end
 
@@ -39,7 +40,8 @@ def from_hash(inventory_objects_data, available_inventory_collections)
       def to_hash
         {
           :name              => name,
-          :manager_uuids     => targeted_scope.values.map { |x| data_to_hash(x) },
+          # TODO(lsmola) we do not support nested references here, should we?
+          :manager_uuids     => targeted_scope.primary_references.values.map(&:full_reference),
           :all_manager_uuids => all_manager_uuids,
           :data              => data.map { |x| data_to_hash(x.data) }
         }
@@ -97,7 +99,7 @@ def hash_to_data(hash, available_inventory_collections, depth = 0)
             hash_to_lazy_relation(value, available_inventory_collections, depth)
           elsif value.kind_of?(Array) && value.first.kind_of?(Hash) && value.first['type'] == "ManagerRefresh::InventoryObjectLazy"
             # TODO(lsmola) do we need to compact it sooner? What if first element is nil? On the other hand, we want to
-            # deprecate Vmthis HABTM assignment because it's not effective
+            # deprecate this Vm HABTM assignment because it's not effective
             value.compact.map { |x| hash_to_lazy_relation(x, available_inventory_collections, depth) }
           else
             value
@@ -114,12 +116,10 @@ def data_to_hash(data, depth = 0)
         raise "Nested lazy_relation of #{inventory_collection} is too deep, left processing: #{data}" if depth > 20
 
         data.transform_values do |value|
-          if inventory_object_lazy?(value)
+          if inventory_object_lazy?(value) || inventory_object?(value)
             lazy_relation_to_hash(value, depth)
           elsif value.kind_of?(Array) && (inventory_object_lazy?(value.compact.first) || inventory_object?(value.compact.first))
             value.compact.map { |x| lazy_relation_to_hash(x, depth) }
-          elsif inventory_object?(value)
-            lazy_relation_to_hash(value, depth)
           else
             value
           end

spec/models/manager_refresh/persister/serializing_spec.rb

@@ -32,11 +32,13 @@
       :hardware       => 3,
       :miq_template   => 1,
       :network        => 2,
-      :vm             => 2,
-      :vm_or_template => 3
+      :vm             => 4,
+      :vm_or_template => 5
     }
+    # 1 Vm will be disconnected
+    ems_counts = counts.dup.merge(:vm => 4, :vm_or_template => 4)
 
-    assert_counts(counts)
+    assert_counts(counts, ems_counts)
 
     vm = Vm.find_by(:ems_ref => "vm_ems_ref_1")
     expect(vm.location).to eq("host_10_10_10_1.com")
@@ -56,9 +58,16 @@
     # we try to evaluate the lazy object. We should be able to do that.
     expect(vm.hardware.model).to eq(nil)
     expect(vm.hardware.manufacturer).to eq(nil)
+
+    expect(Vm.find_by(:ems_ref => "vm_ems_ref_20").ems_id).to be_nil
+    expect(Vm.find_by(:ems_ref => "vm_ems_ref_21").ems_id).not_to be_nil
   end
 
   def populate_test_data(persister)
+    # Add some data into the DB
+    FactoryGirl.create(:vm, vm_data(20))
+    FactoryGirl.create(:vm, vm_data(21))
+
     @image_data_1          = image_data(1)
     @image_hardware_data_1 = image_hardware_data(1).merge(
       :guest_os       => "linux_generic_1",
@@ -118,5 +127,8 @@ def populate_test_data(persister)
     hardware = persister.hardwares.build(hardware_data(2).merge(:vm_or_template => vm))
     persister.networks.build(public_network_data(2).merge(:hardware => hardware))
     persister.disks.build(disk_data(2).merge(:hardware => hardware))
+
+    # Add some targeted_scope
+    persister.vms.targeted_scope << vm_data(20)[:ems_ref]
   end
 end

spec/models/manager_refresh/persister/targeted_refresh_spec_helper.rb

@@ -44,10 +44,13 @@ def base_inventory_counts
     }
   end
 
-  def assert_counts(expected_table_counts)
+  def assert_counts(expected_table_counts, expected_ems_table_counts = nil)
     expected_counts = base_inventory_counts.merge(expected_table_counts)
+    expected_ems_table_counts ||= expected_counts
+    expected_ems_counts = base_inventory_counts.merge(expected_ems_table_counts)
+
     assert_table_counts(expected_counts)
-    assert_ems(expected_counts)
+    assert_ems(expected_ems_counts)
   end
 
   def assert_table_counts(expected_table_counts)