Merge pull request ManageIQ#79 from Ladas/cleanup_unused_code

Cleanup unused code

lib/inventory_refresh.rb

@@ -6,6 +6,4 @@
 require "inventory_refresh/null_logger"
 require "inventory_refresh/persister"
 require "inventory_refresh/save_inventory"
-require "inventory_refresh/target"
-require "inventory_refresh/target_collection"
 require "inventory_refresh/version"

lib/inventory_refresh/inventory_collection.rb

@@ -112,7 +112,6 @@ class InventoryCollection
              :lazy_find_by,
              :named_ref,
              :primary_index,
-             :reindex_secondary_indexes!,
              :skeletal_primary_index,
              :to => :index_proxy
 
@@ -448,13 +447,11 @@ def inspect
 
     # @return [Integer] default batch size for talking to the DB
     def batch_size
-      # TODO(lsmola) mode to the settings
       1000
     end
 
     # @return [Integer] default batch size for talking to the DB if not using ApplicationRecord objects
     def batch_size_pure_sql
-      # TODO(lsmola) mode to the settings
       10_000
     end
 
@@ -569,11 +566,5 @@ def record_identity(record)
         :id => identity
       }
     end
-
-    # TODO: Not used!
-    # @return [Array<Symbol>] all association attributes and foreign keys of the model class
-    def association_attributes
-      model_class.reflect_on_all_associations.map { |x| [x.name, x.foreign_key] }.flatten.compact.map(&:to_sym)
-    end
   end
 end

lib/inventory_refresh/inventory_collection/data_storage.rb

@@ -70,15 +70,6 @@ def find_or_build_by(hash)
         build(hash)
       end
 
-      # Finds InventoryObject.
-      #
-      # @param hash [Hash] Hash that needs to contain attributes defined in :manager_ref of the InventoryCollection
-      # @return [InventoryRefresh::InventoryObject] Found or built InventoryObject object
-      def find_in_data(hash)
-        _hash, _uuid, inventory_object = primary_index_scan(hash)
-        inventory_object
-      end
-
       # Finds of builds a new InventoryObject. By building it, we also put in into the InventoryCollection's storage.
       #
       # @param hash [Hash] Hash that needs to contain attributes defined in :manager_ref of the

lib/inventory_refresh/inventory_collection/index/proxy.rb

@@ -64,14 +64,6 @@ def build_secondary_indexes_for(inventory_object)
           end
         end
 
-        def reindex_secondary_indexes!
-          data_indexes.each do |ref, index|
-            next if ref == primary_index_ref
-
-            index.reindex!
-          end
-        end
-
         def primary_index
           data_index(primary_index_ref)
         end
@@ -94,30 +86,16 @@ def find(reference, ref: primary_index_ref)
           end
         end
 
-        def find_by(manager_uuid_hash, ref: primary_index_ref)
-          # TODO(lsmola) deprecate this, it's enough to have find method
-          find(manager_uuid_hash, :ref => ref)
-        end
-
-        def lazy_find_by(manager_uuid_hash, ref: primary_index_ref, key: nil, default: nil)
-          # TODO(lsmola) deprecate this, it's enough to have lazy_find method
-
-          lazy_find(manager_uuid_hash, :ref => ref, :key => key, :default => default)
-        end
-
         def lazy_find(manager_uuid, ref: primary_index_ref, key: nil, default: nil, transform_nested_lazy_finds: false)
-          # TODO(lsmola) also, it should be enough to have only 1 find method, everything can be lazy, until we try to
-          # access the data
-          # TODO(lsmola) lazy_find will support only hash, then we can remove the _by variant
           return if manager_uuid.nil?
           assert_index(manager_uuid, ref)
 
           ::InventoryRefresh::InventoryObjectLazy.new(inventory_collection,
-                                                    manager_uuid,
-                                                    :ref                         => ref,
-                                                    :key                         => key,
-                                                    :default                     => default,
-                                                    :transform_nested_lazy_finds => transform_nested_lazy_finds)
+                                                      manager_uuid,
+                                                      :ref                         => ref,
+                                                      :key                         => key,
+                                                      :default                     => default,
+                                                      :transform_nested_lazy_finds => transform_nested_lazy_finds)
         end
 
         def named_ref(ref = primary_index_ref)

lib/inventory_refresh/inventory_collection/index/type/base.rb

@@ -27,14 +27,6 @@ def store_index_for(inventory_object)
             index[build_stringified_reference(inventory_object.data, attribute_names)] = inventory_object
           end
 
-          # Rebuilds the indexes for all InventoryObject objects
-          def reindex!
-            self.index = {}
-            data.each do |inventory_object|
-              store_index_for(inventory_object)
-            end
-          end
-
           # @return [Array] Returns index data
           def index_data
             index.values

lib/inventory_refresh/inventory_collection/references_storage.rb

@@ -45,23 +45,6 @@ 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 [InventoryRefresh::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 => InventoryRefresh::InventoryCollection::Reference}] Hash of indexed Reference objects
-      def primary_references
-        references[primary_index_ref]
-      end
-
       # Builds a Reference object
       #
       # @param reference_data [InventoryRefresh::InventoryCollection::References, Hash, Object] Either existing Reference

lib/inventory_refresh/inventory_collection/serialization.rb

@@ -107,10 +107,6 @@ def hash_to_data(hash, available_inventory_collections, depth = 0)
         hash.transform_values do |value|
           if value.kind_of?(Hash) && value['inventory_collection_name']
             hash_to_lazy_relation(value, available_inventory_collections, depth)
-          elsif value.kind_of?(Array) && value.first.kind_of?(Hash) && value.first['inventory_collection_name']
-            # TODO(lsmola) do we need to compact it sooner? What if first element is nil? On the other hand, we want to
-            # 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
           end
@@ -128,8 +124,6 @@ def data_to_hash(data, depth = 0)
         data.transform_values do |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) }
           else
             value
           end

lib/inventory_refresh/inventory_object.rb

@@ -39,59 +39,6 @@ def key
       nil
     end
 
-    # Transforms InventoryObject object data into hash format with keys that are column names and resolves correct
-    # values of the foreign keys (even the polymorphic ones)
-    #
-    # @param inventory_collection_scope [InventoryRefresh::InventoryCollection] parent InventoryCollection object
-    # @return [Hash] Data in DB format
-    def attributes(inventory_collection_scope = nil)
-      # We should explicitly pass a scope, since the inventory_object can be mapped to more InventoryCollections with
-      # different blacklist and whitelist. The generic code always passes a scope.
-      inventory_collection_scope ||= inventory_collection
-
-      attributes_for_saving = {}
-      # First transform the values
-      data.each do |key, value|
-        if !allowed?(inventory_collection_scope, key)
-          next
-        elsif value.kind_of?(Array) && value.any? { |x| loadable?(x) }
-          # Lets fill also the original data, so other InventoryObject referring to this attribute gets the right
-          # result
-          data[key] = value.compact.map(&:load).compact
-          # We can use built in _ids methods to assign array of ids into has_many relations. So e.g. the :key_pairs=
-          # relation setter will become :key_pair_ids=
-          attributes_for_saving[(key.to_s.singularize + "_ids").to_sym] = data[key].map(&:id).compact.uniq
-        elsif loadable?(value) || inventory_collection_scope.association_to_foreign_key_mapping[key]
-          # Lets fill also the original data, so other InventoryObject referring to this attribute gets the right
-          # result
-          data[key] = value.load if value.respond_to?(:load)
-          if (foreign_key = inventory_collection_scope.association_to_foreign_key_mapping[key])
-            # We have an association to fill, lets fill also the :key, cause some other InventoryObject can refer to it
-            record_id                                 = data[key].try(:id)
-            attributes_for_saving[foreign_key.to_sym] = record_id
-
-            if (foreign_type = inventory_collection_scope.association_to_foreign_type_mapping[key])
-              # If we have a polymorphic association, we need to also fill a base class name, but we want to nullify it
-              # if record_id is missing
-              base_class = data[key].try(:base_class_name) || data[key].class.try(:base_class).try(:name)
-              attributes_for_saving[foreign_type.to_sym] = record_id ? base_class : nil
-            end
-          elsif data[key].kind_of?(::InventoryRefresh::InventoryObject)
-            # We have an association to fill but not an Activerecord association, so e.g. Ancestry, lets just load
-            # it here. This way of storing ancestry is ineffective in DB call count, but RAM friendly
-            attributes_for_saving[key.to_sym] = data[key].base_class_name.constantize.find_by(:id => data[key].id)
-          else
-            # We have a normal attribute to fill
-            attributes_for_saving[key.to_sym] = data[key]
-          end
-        else
-          attributes_for_saving[key.to_sym] = value
-        end
-      end
-
-      attributes_for_saving
-    end
-
     # Transforms InventoryObject object data into hash format with keys that are column names and resolves correct
     # values of the foreign keys (even the polymorphic ones)
     #

lib/inventory_refresh/inventory_object_lazy.rb

@@ -30,7 +30,6 @@ def initialize(inventory_collection, index_data, ref: :manager_ref, key: nil, de
 
     # @return [String] stringified reference
     def to_s
-      # TODO(lsmola) do we need this method?
       stringified_reference
     end
 
@@ -71,10 +70,6 @@ def transitive_dependency?
 
     # @return [Boolean] true if the key is an association on inventory_collection_scope model class
     def association?(key)
-      # TODO(lsmola) remove this if there will be better dependency scan, probably with transitive dependencies filled
-      # in a second pass, then we can get rid of this hardcoded symbols. Right now we are not able to introspect these.
-      return true if [:parent, :genealogy_parent].include?(key)
-
       inventory_collection.dependency_attributes.key?(key) ||
         !inventory_collection.association_to_foreign_key_mapping[key].nil?
     end

lib/inventory_refresh/persister.rb

@@ -3,15 +3,13 @@ class Persister
     require 'json'
     require 'yaml'
 
-    attr_reader :manager, :target, :collections
+    attr_reader :manager, :collections
 
     attr_accessor :refresh_state_uuid, :refresh_state_part_uuid, :total_parts, :sweep_scope, :retry_count, :retry_max
 
     # @param manager [ManageIQ::Providers::BaseManager] A manager object
-    # @param target [Object] A refresh Target object
-    def initialize(manager, target = nil)
+    def initialize(manager)
       @manager = manager
-      @target  = target
 
       @collections = {}
 
@@ -122,18 +120,16 @@ class << self
       #
       # @param json_data [String] input JSON data
       # @return [ManageIQ::Providers::Inventory::Persister] Persister object loaded from a passed JSON
-      def from_json(json_data, manager, target = nil)
-        from_hash(JSON.parse(json_data), manager, target)
+      def from_json(json_data, manager)
+        from_hash(JSON.parse(json_data), manager)
       end
 
       # Returns Persister object built from serialized data
       #
       # @param persister_data [Hash] serialized Persister object in hash
       # @return [ManageIQ::Providers::Inventory::Persister] Persister object built from serialized data
-      def from_hash(persister_data, manager, target = nil)
-        target ||= InventoryRefresh::TargetCollection.new(:manager => manager)
-
-        new(manager, target).tap do |persister|
+      def from_hash(persister_data, manager)
+        new(manager).tap do |persister|
           persister_data['collections'].each do |collection|
             inventory_collection = persister.collections[collection['name'].try(:to_sym)]
             raise "Unrecognized InventoryCollection name: #{collection['name']}" if inventory_collection.blank?

lib/inventory_refresh/save_collection/saver/sql_helper_upsert.rb

@@ -92,8 +92,6 @@ def insert_query_on_conflict_update(all_attribute_keys, mode, ignore_cols, colum
                               :resource_counter
                             end
 
-        # TODO(lsmola) should we add :deleted => false to the update clause? That should handle a reconnect, without a
-        # a need to list :deleted anywhere in the parser. We just need to check that a model has the :deleted attribute
         query = <<-SQL
           SET #{(all_attribute_keys - ignore_cols).map { |key| build_insert_set_cols(key) }.join(", ")}
         SQL