Skip to content

Commit

Permalink
[WIP] Use OpenStruct over MiqHashStruct
Browse files Browse the repository at this point in the history
This is almost a straight 1 for 1 conversion, and performs much better
than our implementation: MiqHashStruct.

* * *

Regarding the [WIP] portion:

The only part that doesn't work (but also doesn't make any sense that it
ever did), was the portion of this code that was commented out:

    - if tz[1].to_i_with_method == val.to_i_with_method
    -   # Save [value, description] for timezones array
    -   init_values[field_name] = [val, tz[0]]
    - end
    + # if tz[1].to_i_with_method == val.to_i_with_method
    + #   # Save [value, description] for timezones array
    + #   init_values[field_name] = [val, tz[0]]
    + # end

`tz` in the above always seems to be a MiqHashStruct (in the previous
commit), and when `tz[1]` is called, it hit's `method_missing` with
`:[]` as the method argument.  This of course will almost never have a
match in the underlying `@hash` in the MiqHashStruct, but will not blow
up at least.

When it is an OpenStruct, this fails with an error since `1` can't be
converted to a symbol properly.  I am uncertain if this code is ever
activated, so for now, I have just commented it out to observe the
advantages to using OpenStruct over MiqHashStruct.
  • Loading branch information
NickLaMuro committed Apr 27, 2018
1 parent 7f77ae9 commit ef7672b
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 20 deletions.
6 changes: 3 additions & 3 deletions app/models/miq_host_provision_workflow.rb
Original file line number Diff line number Diff line change
Expand Up @@ -207,15 +207,15 @@ def ws_find_matching_ci(allowed_method, keys, match_str, klass)
end

def self.from_ws(*args)
# Move optional arguments into the MiqHashStruct object
# Move optional arguments into the OpenStruct object
prov_args = args[0, 6]
prov_options = MiqHashStruct.new(:values => args[6], :ems_custom_attributes => args[7], :miq_custom_attributes => args[8])
prov_options = OpenStruct.new(:values => args[6], :ems_custom_attributes => args[7], :miq_custom_attributes => args[8])
prov_args << prov_options
MiqHostProvisionWorkflow.from_ws_ver_1_x(*prov_args)
end

def self.from_ws_ver_1_x(version, user, template_fields, vm_fields, requester, tags, options)
options = MiqHashStruct.new if options.nil?
options = OpenStruct.new if options.nil?
_log.warn("Web-service host provisioning starting with interface version <#{version}> by requester <#{user.userid}>")

init_options = {:use_pre_dialog => false, :request_type => request_type(parse_ws_string(template_fields)[:request_type])}
Expand Down
12 changes: 6 additions & 6 deletions app/models/miq_provision_virt_workflow.rb
Original file line number Diff line number Diff line change
Expand Up @@ -724,8 +724,8 @@ def self.from_ws(*args)
version = args.first.to_f
return from_ws_ver_1_0(*args) if version == 1.0

# Move optional arguments into the MiqHashStruct object
prov_options = MiqHashStruct.new(
# Move optional arguments into the OpenStruct object
prov_options = OpenStruct.new(
:values => args[6],
:ems_custom_attributes => args[7],
:miq_custom_attributes => args[8],
Expand Down Expand Up @@ -938,7 +938,7 @@ def ws_customize_fields(values, _fields, data)
end

def self.from_ws_ver_1_x(version, user, template_fields, vm_fields, requester, tags, options)
options = MiqHashStruct.new if options.nil?
options = OpenStruct.new if options.nil?
_log.warn("Web-service provisioning starting with interface version <#{version}> by requester <#{user.userid}>")

init_options = {:use_pre_dialog => false, :request_type => request_type(parse_ws_string(template_fields)[:request_type]), :initial_pass => true}
Expand Down Expand Up @@ -1045,11 +1045,11 @@ def create_hash_struct_from_vm_or_template(vm_or_template, options)
:v_total_snapshots => vm_or_template.v_total_snapshots,
:evm_object_class => :Vm}
data_hash[:cloud_tenant] = vm_or_template.cloud_tenant if vm_or_template.respond_to?(:cloud_tenant)
hash_struct = MiqHashStruct.new(data_hash)
hash_struct.operating_system = MiqHashStruct.new(
hash_struct = OpenStruct.new(data_hash)
hash_struct.operating_system = OpenStruct.new(
:product_name => vm_or_template.operating_system.product_name
) if vm_or_template.operating_system
hash_struct.ext_management_system = MiqHashStruct.new(
hash_struct.ext_management_system = OpenStruct.new(
:name => vm_or_template.ext_management_system.name
) if vm_or_template.ext_management_system
if options[:include_datacenter] == true
Expand Down
26 changes: 15 additions & 11 deletions app/models/miq_request_workflow.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
require 'enumerator'
require 'miq-hash_struct'
require 'ostruct'

class MiqRequestWorkflow
include Vmdb::Logging
Expand Down Expand Up @@ -113,10 +113,10 @@ def init_from_dialog(init_values)
init_values[field_name] = [val, field_values[:values][val]]
else
field_values[:values].each do |tz|
if tz[1].to_i_with_method == val.to_i_with_method
# Save [value, description] for timezones array
init_values[field_name] = [val, tz[0]]
end
# if tz[1].to_i_with_method == val.to_i_with_method
# # Save [value, description] for timezones array
# init_values[field_name] = [val, tz[0]]
# end
end
end
else
Expand Down Expand Up @@ -641,7 +641,7 @@ def tag_symbol
end

def build_ci_hash_struct(ci, props)
nh = MiqHashStruct.new(:id => ci.id, :evm_object_class => ci.class.base_class.name.to_sym)
nh = OpenStruct.new(:id => ci.id, :evm_object_class => ci.class.base_class.name.to_sym)
props.each { |p| nh.send("#{p}=", ci.send(p)) }
nh
end
Expand Down Expand Up @@ -842,15 +842,19 @@ def find_classes_under_ci(item, klass)

def load_ems_node(item, log_header)
@ems_xml_nodes ||= {}
klass_name = item.kind_of?(MiqHashStruct) ? item.evm_object_class : item.class.base_class.name
klass_name = if item.kind_of?(OpenStruct)
Object.const_get(item.evm_object_class)
else
item.class.base_class
end
node = @ems_xml_nodes["#{klass_name}_#{item.id}"]
$log.error("#{log_header} Resource <#{klass_name}_#{item.id} - #{item.name}> not found in cached resource tree.") if node.nil?
node
end

def ems_has_clusters?
found = each_ems_metadata(nil, EmsCluster) { |ci| break(ci) }
return found.evm_object_class == :EmsCluster if found.kind_of?(MiqHashStruct)
return found.evm_object_class == :EmsCluster if found.kind_of?(OpenStruct)
false
end

Expand Down Expand Up @@ -1008,7 +1012,7 @@ def customization_spec_to_hash_struct(ci)

def load_ar_obj(ci)
return load_ar_objs(ci) if ci.kind_of?(Array)
return ci unless ci.kind_of?(MiqHashStruct)
return ci unless ci.kind_of?(OpenStruct)
ci.evm_object_class.to_s.camelize.constantize.find_by(:id => ci.id)
end

Expand Down Expand Up @@ -1227,7 +1231,7 @@ def set_ws_field_value(values, key, data, dialog_name, dlg_fields)
field_values = dlg_field[:values]
_log.info("processing key <#{dialog_name}:#{key}(#{data_type})> with values <#{field_values.inspect}>")
if field_values.present?
result = if field_values.first.kind_of?(MiqHashStruct)
result = if field_values.first.kind_of?(OpenStruct)
found = field_values.detect { |v| v.id == set_value }
[found.id, found.name] if found
elsif data_type == :array_integer
Expand Down Expand Up @@ -1268,7 +1272,7 @@ def set_ws_field_value_by_display_name(values, key, data, dialog_name, dlg_field
field_values = dlg_field[:values]
_log.info("processing key <#{dialog_name}:#{key}(#{data_type})> with values <#{field_values.inspect}>")
if field_values.present?
result = if field_values.first.kind_of?(MiqHashStruct)
result = if field_values.first.kind_of?(OpenStruct)
found = field_values.detect { |v| v.send(obj_key).to_s.downcase == find_value }
[found.id, found.send(obj_key)] if found
else
Expand Down

0 comments on commit ef7672b

Please sign in to comment.