Merge pull request #13926 from Ladas/introducing_find_by_and_find_or_…

…build_by_methods

Introducing find by and find or build by methods

app/models/manageiq/providers/ansible_tower/inventory/parser/automation_manager.rb

@@ -20,6 +20,7 @@ def configured_systems
       inventory_object.hostname = host.name
       inventory_object.virtual_instance_ref = host.instance_id
       inventory_object.inventory_root_group = persister.inventory_root_groups.lazy_find(host.inventory_id.to_s)
+      # TODO(lsmola) get rid of direct DB access
       inventory_object.counterpart = Vm.find_by(:uid_ems => host.instance_id)
     end
   end
@@ -52,9 +53,10 @@ def configuration_script_sources
       inventory_object.name = project.name
 
       project.playbooks.each do |playbook_name|
-        # FIXME: its not really nice how I have to build a manager_ref / uuid here
-        inventory_object_playbook = persister.configuration_script_payloads.find_or_build("#{project.id}__#{playbook_name}")
-        inventory_object_playbook.configuration_script_source = inventory_object
+        inventory_object_playbook = persister.configuration_script_payloads.find_or_build_by(
+          :configuration_script_source => inventory_object,
+          :manager_ref                 => playbook_name
+        )
         inventory_object_playbook.name = playbook_name
       end
     end

app/models/manageiq/providers/ansible_tower/inventory_collection_default/automation_manager.rb

@@ -39,7 +39,7 @@ def configuration_script_payloads(extra_attributes = {})
       attributes = {
         :model_class => ManageIQ::Providers::AnsibleTower::AutomationManager::Playbook,
         :association => :configuration_script_payloads,
-        :manager_ref => [:manager_ref],
+        :manager_ref => [:configuration_script_source, :manager_ref],
       }
       attributes.merge!(extra_attributes)
     end

app/models/manager_refresh/inventory_collection.rb

@@ -126,9 +126,18 @@ def <<(inventory_object)
     alias push <<
 
     def object_index(object)
-      stringify_reference(
-        manager_ref.map { |attribute| object.public_send(attribute).try(:id) || object.public_send(attribute).to_s }
-      )
+      index_array = manager_ref.map do |attribute|
+        if object.respond_to?(:[])
+          object[attribute].to_s
+        else
+          object.public_send(attribute).try(:id) || object.public_send(attribute).to_s
+        end
+      end
+      stringify_reference(index_array)
+    end
+
+    def object_index_with_keys(keys, object)
+      keys.map { |attribute| object.public_send(attribute).to_s }.join(stringify_joiner)
     end
 
     def stringify_joiner
@@ -146,13 +155,20 @@ def manager_ref_to_cols
       end
     end
 
-    def object_index_with_keys(keys, object)
-      keys.map { |attribute| object.public_send(attribute).to_s }.join(stringify_joiner)
+    def find_or_build(manager_uuid)
+      raise "The uuid consists of #{manager_ref.size} attributes, please find_or_build_by method" if manager_ref.size > 1
+
+      find_or_build_by(manager_ref.first => manager_uuid)
     end
 
-    def find_or_build(manager_uuid)
-      # FIXME: splat manager_ref
-      data_index[manager_uuid] || build(manager_ref.first => manager_uuid)
+    def find_or_build_by(manager_uuid_hash)
+      if !manager_uuid_hash.keys.all? { |x| manager_ref.include?(x) } || manager_uuid_hash.keys.size != manager_ref.size
+        raise "Allowed find_or_build_by keys are #{manager_ref}"
+      end
+
+      # Not using find by since if could take record from db, then any changes would be ignored, since such record will
+      # not be stored to DB, maybe we should rethink this?
+      data_index[object_index(manager_uuid_hash)] || build(manager_uuid_hash)
     end
 
     def find(manager_uuid)
@@ -167,6 +183,13 @@ def find(manager_uuid)
       end
     end
 
+    def find_by(manager_uuid_hash)
+      if !manager_uuid_hash.keys.all? { |x| manager_ref.include?(x) } || manager_uuid_hash.keys.size != manager_ref.size
+        raise "Allowed find_by keys are #{manager_ref}"
+      end
+      find(object_index(manager_uuid_hash))
+    end
+
     def find_in_db(manager_uuid)
       manager_uuids    = manager_uuid.split(stringify_joiner)
       hash_uuid_by_ref = manager_ref.zip(manager_uuids).to_h

app/models/manager_refresh/inventory_object.rb

@@ -74,11 +74,11 @@ def attributes(inventory_collection_scope = nil)
     end
 
     def to_s
-      "InventoryObject:('#{manager_uuid}', #{inventory_collection})"
+      manager_uuid
     end
 
     def inspect
-      to_s
+      "InventoryObject:('#{manager_uuid}', #{inventory_collection})"
     end
 
     def dependency?