Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested lazy find with secondary ref #17425

Merged
merged 8 commits into from
May 16, 2018
21 changes: 18 additions & 3 deletions app/models/manager_refresh/inventory_collection/data_storage.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ class DataStorage
:skeletal_primary_index,
:to => :index_proxy

delegate :builder_params,
delegate :association_to_foreign_key_mapping,
:builder_params,
:inventory_object?,
:inventory_object_lazy?,
:manager_ref,
Expand Down Expand Up @@ -133,12 +134,26 @@ def from_hash(inventory_objects_data, available_inventory_collections)
def primary_index_scan(hash)
hash = enrich_data(hash)

assert_all_keys_present(hash)
assert_only_primary_index(hash)

uuid = ::ManagerRefresh::InventoryCollection::Reference.build_stringified_reference(hash, named_ref)
return hash, uuid, primary_index.find(uuid)
end

def assert_all_keys_present(hash)
if manager_ref.any? { |x| !hash.key?(x) }
raise "Needed find_or_build_by keys are: #{manager_ref}, data provided: #{hash}"
end
end

uuid = ::ManagerRefresh::InventoryCollection::Reference.build_stringified_reference(hash, named_ref)
return hash, uuid, primary_index.find(uuid)
def assert_only_primary_index(data)
named_ref.each do |key|
if data[key].kind_of?(ManagerRefresh::InventoryObjectLazy) && !data[key].primary?
raise "Wrong index for key :#{key}, all references under this index must point to default :ref called"\
" :manager_ref. Any other :ref is not valid. This applies also to nested lazy links."
end
end
end

# Returns new hash enriched by (see ManagerRefresh::InventoryCollection#builder_params) hash
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ def initialize(inventory_collection, secondary_refs = {})
# @return [ManagerRefresh::InventoryObject] Passed InventoryObject
def build_primary_index_for(inventory_object)
# Building the object, we need to provide all keys of a primary index

assert_index(inventory_object.data, primary_index_ref)
primary_index.store_index_for(inventory_object)
end
Expand Down Expand Up @@ -98,7 +99,7 @@ def lazy_find_by(manager_uuid_hash, ref: primary_index_ref, key: nil, default: n
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)
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
Expand All @@ -107,7 +108,10 @@ def lazy_find(manager_uuid, ref: primary_index_ref, key: nil, default: nil)

::ManagerRefresh::InventoryObjectLazy.new(inventory_collection,
manager_uuid,
:ref => ref, :key => key, :default => default)
:ref => ref,
:key => key,
:default => default,
:transform_nested_lazy_finds => transform_nested_lazy_finds)
end

def named_ref(ref = primary_index_ref)
Expand Down
19 changes: 16 additions & 3 deletions app/models/manager_refresh/inventory_collection/reference.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,23 @@ def initialize(data, ref, keys)
@ref = ref
@keys = keys

@nested_secondary_index = keys.select { |key| full_reference[key].kind_of?(ManagerRefresh::InventoryObjectLazy) }.any? do |key|
!full_reference[key].primary?
end

@stringified_reference = self.class.build_stringified_reference(full_reference, keys)
end

# Return true if reference is to primary index, false otherwise.
# Return true if reference is to primary index, false otherwise. Reference is primary, only if all the nested
# references are primary.
#
# @return [Boolean] true if reference is to primary index, false otherwise
def primary?
ref == :manager_ref
ref == :manager_ref && !nested_secondary_index
end

def nested_secondary_index?
nested_secondary_index
end

class << self
Expand Down Expand Up @@ -57,14 +66,18 @@ def stringify_reference(reference)

# Returns joiner for string UIID
#
# @return [String] Joiner for stirng UIID
# @return [String] Joiner for string UIID
def stringify_joiner
"__"
end
end

private

# @return Returns true if reference has nested references that are not pointing to primary index, but to
# secondary indexes.
attr_reader :nested_secondary_index

# Returns original Hash, or build hash out of passed string
#
# @param data [Hash, String] Passed data
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,13 @@ def to_hash
# @return [Hash] Serialized ManagerRefresh::InventoryObjectLazy
def lazy_relation_to_hash(value, depth = 0)
{
:type => "ManagerRefresh::InventoryObjectLazy",
:inventory_collection_name => value.inventory_collection.name,
:reference => data_to_hash(value.reference.full_reference, depth + 1),
:ref => value.reference.ref,
:key => value.try(:key),
:default => value.try(:default),
:type => "ManagerRefresh::InventoryObjectLazy",
:inventory_collection_name => value.inventory_collection.name,
:reference => data_to_hash(value.reference.full_reference, depth + 1),
:ref => value.reference.ref,
:key => value.try(:key),
:default => value.try(:default),
:transform_nested_lazy_finds => value.try(:transform_nested_lazy_finds)
}
end

Expand All @@ -78,9 +79,10 @@ def hash_to_lazy_relation(value, available_inventory_collections, depth = 0)

inventory_collection.lazy_find(
hash_to_data(value['reference'], available_inventory_collections, depth + 1).symbolize_keys!,
:ref => value['ref'].try(:to_sym),
:key => value['key'].try(:to_sym),
:default => value['default']
:ref => value['ref'].try(:to_sym),
:key => value['key'].try(:to_sym),
:default => value['default'],
:transform_nested_lazy_finds => value['transform_nested_lazy_finds']
)
end

Expand Down
34 changes: 31 additions & 3 deletions app/models/manager_refresh/inventory_object_lazy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ module ManagerRefresh
class InventoryObjectLazy
include Vmdb::Logging

attr_reader :reference, :inventory_collection, :key, :default
attr_reader :reference, :inventory_collection, :key, :default, :transform_nested_lazy_finds

delegate :stringified_reference, :ref, :[], :to => :reference

Expand All @@ -12,12 +12,17 @@ class InventoryObjectLazy
# @param ref [Symbol] reference name
# @param key [Symbol] key name, will be used to fetch attribute from resolved InventoryObject
# @param default [Object] a default value used if the :key will resolve to nil
def initialize(inventory_collection, index_data, ref: :manager_ref, key: nil, default: nil)
# @param transform_nested_lazy_finds [Boolean] True if we want to convert all lazy objects in InventoryObject
# objects and reset the Reference. TODO(lsmola) we should be able to do this automatically, then we can
# remove this option
def initialize(inventory_collection, index_data, ref: :manager_ref, key: nil, default: nil, transform_nested_lazy_finds: false)
@inventory_collection = inventory_collection
@reference = inventory_collection.build_reference(index_data, ref)
@key = key
@default = default

@transform_nested_lazy_finds = transform_nested_lazy_finds

# We do not support skeletal pre-create for :key, since :key will not be available, we want to use local_db_find
# instead.
skeletal_precreate! unless @key
Expand All @@ -40,6 +45,8 @@ def inspect
# @return [ManagerRefresh::InventoryObject, Object] ManagerRefresh::InventoryObject instance or an attribute
# on key
def load
transform_nested_secondary_indexes! if transform_nested_lazy_finds && nested_secondary_index?

key ? load_object_with_key : load_object
end

Expand Down Expand Up @@ -70,10 +77,31 @@ def association?(key)
!inventory_collection.association_to_foreign_key_mapping[key].nil?
end

def transform_nested_secondary_indexes!(depth = 0)
raise "Nested references are too deep!" if depth > 20

keys.each do |x|
attr = full_reference[x]
next unless attr.kind_of?(ManagerRefresh::InventoryObjectLazy)
next if attr.primary?

if attr.nested_secondary_index?
attr.transform_nested_secondary_indexes!(depth + 1)
end

full_reference[x] = full_reference[x].load
end

# Rebuild the reference to get the right value
self.reference = inventory_collection.build_reference(full_reference, ref)
end

private

delegate :parallel_safe?, :saved?, :saver_strategy, :skeletal_primary_index, :targeted?, :to => :inventory_collection
delegate :full_reference, :keys, :primary?, :to => :reference
delegate :nested_secondary_index?, :primary?, :full_reference, :keys, :primary?, :to => :reference

attr_writer :reference

# Instead of loading the reference from the DB, we'll add the skeletal InventoryObject (having manager_ref and
# info from the builder_params) to the correct InventoryCollection. Which will either be found in the DB or
Expand Down
27 changes: 27 additions & 0 deletions spec/models/manager_refresh/persister/finders_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -148,4 +148,31 @@
it "checks find_or_build_by finds existing inventory object instead of duplicating" do
expect(persister.vms.find_or_build_by(vm_data(1)).object_id).to eq(persister.vms.find_or_build_by(vm_data(1)).object_id)
end

it "raises exception unless only primary index is used in nested lazy_find when building" do
name = vm_data(1)[:name]
vm_lazy = persister.vms.lazy_find({:name => name}, :ref => :by_name)

persister.vms.build(vm_data(1))

expected_error = "Wrong index for key :vm_or_template, all references under this index must point to default :ref"\
" called :manager_ref. Any other :ref is not valid. This applies also to nested lazy links."
expect do
persister.hardwares.build(hardware_data(1, :vm_or_template => vm_lazy))
end.to(raise_error(expected_error))
end

it "raises exception unless only primary index is used in deep nested lazy_find when building" do
name = vm_data(1)[:name]
vm_lazy = persister.vms.lazy_find({:name => name}, :ref => :by_name)
hardware_lazy = persister.hardwares.lazy_find(:vm_or_template => vm_lazy)

persister.vms.build(vm_data(1))

expected_error = "Wrong index for key :hardware, all references under this index must point to default :ref called"\
" :manager_ref. Any other :ref is not valid. This applies also to nested lazy links."
expect do
persister.disks.build(disk_data(1, :hardware => hardware_lazy))
end.to(raise_error(expected_error))
end
end
29 changes: 20 additions & 9 deletions spec/models/manager_refresh/persister/serializing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,8 @@
expect(vm2.hardware.networks.first.ipaddress).to eq("10.10.10.2")
expect(vm2.hardware.disks.first.device_name).to eq("disk_name_2")

# TODO(lsmola) interestingly persister.hardwares.lazy_find({:vm_or_template => lazy_find_image_sec}) will not work
# because we build the reference only from the passed data. To make it work, we would have to do internally
# persister.hardwares.lazy_find({:vm_or_template => lazy_find_image_sec.load}), which will allow the nested lazy
# find to reach its :manager_ref data. That would mean the stringified reference would have to be build when
# 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.hardware.model).to eq("test1")
expect(vm.hardware.manufacturer).to eq("test2")

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
Expand All @@ -82,6 +77,20 @@ def populate_test_data(persister)
lazy_find_image_sec1 = persister.miq_templates.lazy_find(
{:name => image_data(1)[:name], :uid_ems => image_data(1)[:uid_ems]}, {:ref => :by_uid_ems_and_name}
)
# TODO(lsmola) we build :hardwares with vm_or_template => persister.miq_templates.lazy_find(:ems_ref => image_data(1)[:ems_ref])
# so it's indexed by vm ems_ref. Then we try to fetch it with lazy_find_image_sec2, which uses a by_uid_ems_and_name.
# So we can convert that to ems_ref, but what if we would build the :hardware with by_uid_ems_and_name? We do this
# in k8s provider
#
# We will need to store in hardware IC what it was build with, then we will know what we can convert it to? And hopefully
# we will not build the hardware with different :keys? But we might be actually doing it and I think it's fine for
# creating? But it sucks for lazy_find, maybe we will just forbid it?
#
# Or we will store what attributes were used to build hardware, then we can try to find it based on all of them?
# Then we would also transform the current lazy_object on find? But it would not work with current recursion.
#
# And we need more focus on having e.g. hardware, using vm with different refs, since we can then have duplicates.
# On saving, those duplicates will be eliminated, but if anything pointed to the duplicate, it will not get it's id.
lazy_find_image_sec2 = persister.miq_templates.lazy_find(
{:name => image_data(1)[:name], :uid_ems => image_data(1)[:uid_ems]}, {:ref => :by_uid_ems_and_name, :key => :vendor}
)
Expand All @@ -101,8 +110,10 @@ def populate_test_data(persister)

@hardware_data_1 = hardware_data(1).merge(
:guest_os => persister.hardwares.lazy_find({:vm_or_template => lazy_find_image}, {:key => :guest_os}),
:model => persister.hardwares.lazy_find({:vm_or_template => lazy_find_image_sec}, {:key => :model}),
:manufacturer => persister.hardwares.lazy_find({:vm_or_template => lazy_find_image_sec1}, {:key => :manufacturer}),
:model => persister.hardwares.lazy_find({:vm_or_template => lazy_find_image_sec},
{:key => :model, :transform_nested_lazy_finds => true}),
:manufacturer => persister.hardwares.lazy_find({:vm_or_template => lazy_find_image_sec1},
{:key => :manufacturer, :transform_nested_lazy_finds => true}),
:guest_os_full_name => lazy_find_image_sec2,
:vm_or_template => persister.vms.lazy_find(:ems_ref => vm_data(1)[:ems_ref])
)
Expand Down