Skip to content

Commit

Permalink
Merge pull request #17629 from NickLaMuro/custom_attributes_into_extr…
Browse files Browse the repository at this point in the history
…a_cols_for_miq_reports_gapindashvili

[GAPRINDASHVILI] Use AREL for custom_attributes virtual_attributes in MiqReport ONLY
(cherry picked from commit d7d0f96)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1594028
  • Loading branch information
simaishi committed Jun 26, 2018
1 parent 44dd61a commit cb5a8cb
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 7 deletions.
7 changes: 7 additions & 0 deletions app/models/miq_report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,13 @@ def load_custom_attributes
klass.load_custom_attributes_for(cols.uniq)
end

def select_custom_attributes_for(cols)
klass = db.safe_constantize
return unless klass < CustomAttributeMixin

klass.select_custom_attributes_for(cols)
end

# this method adds :custom_attributes => {} to MiqReport#include
# when report with virtual custom attributes is stored
# we need preload custom_attributes table to main query for building report for elimination of superfluous queries
Expand Down
17 changes: 16 additions & 1 deletion app/models/miq_report/generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,17 @@ def _generate_table(options = {})
va_sql_cols = cols.select do |col|
db_class.virtual_attribute?(col) && db_class.attribute_supported_by_sql?(col)
end

custom_attributes_sql_cols = select_custom_attributes_for(cols)

# Can't use `remove_loading_relations_for_virtual_custom_attributes`
# since `includes` is what is being passed, and not the `include`
# instance variable which is modified in the method.
if custom_attributes_sql_cols.present?
vc_attributes = CustomAttributeMixin.select_virtual_custom_attributes(cols).present?
includes.delete(:custom_attributes) if vc_attributes.present? && includes && includes[:custom_attributes].blank?
end

rbac_opts = options.merge(
:targets => targets,
:filter => conditions,
Expand All @@ -299,7 +310,11 @@ def _generate_table(options = {})
:skip_counts => true,
)

rbac_opts[:extra_cols] = va_sql_cols unless va_sql_cols.nil? || va_sql_cols.empty?
if va_sql_cols.present? || custom_attributes_sql_cols.present?
rbac_opts[:extra_cols] ||= []
rbac_opts[:extra_cols] += va_sql_cols if va_sql_cols.present?
rbac_opts[:extra_cols] += custom_attributes_sql_cols if custom_attributes_sql_cols.present?
end

results, attrs = Rbac.search(rbac_opts)
results = Metric::Helper.remove_duplicate_timestamps(results)
Expand Down
42 changes: 36 additions & 6 deletions app/models/mixins/custom_attribute_mixin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,22 +38,52 @@ def self.load_custom_attributes_for(cols)
custom_attributes.each { |custom_attribute| add_custom_attribute(custom_attribute) }
end

def self.select_custom_attributes_for(cols)
custom_attributes = CustomAttributeMixin.select_virtual_custom_attributes(cols)
custom_attributes.map do |custom_attribute|
without_prefix = custom_attribute.sub(CUSTOM_ATTRIBUTES_PREFIX, "")
name_val, section = without_prefix.split(SECTION_SEPARATOR)
sanatized_column_alias = custom_attribute.tr('.', 'DOT').tr('/', 'BS').tr(':', 'CLN')

custom_attribute_arel(name_val, section, sanatized_column_alias)
end
end

def self.add_custom_attribute(custom_attribute)
return if respond_to?(custom_attribute)

virtual_column(custom_attribute.to_sym, :type => :string, :uses => :custom_attributes)
ca_sym = custom_attribute.to_sym
without_prefix = custom_attribute.sub(CUSTOM_ATTRIBUTES_PREFIX, "")
name_val, section = without_prefix.split(SECTION_SEPARATOR)
sanatized_column_alias = custom_attribute.tr('.', 'DOT').tr('/', 'BS').tr(':', 'CLN')

define_method(custom_attribute.to_sym) do
custom_attribute_without_prefix = custom_attribute.sub(CUSTOM_ATTRIBUTES_PREFIX, "")
custom_attribute_without_section, section = custom_attribute_without_prefix.split(SECTION_SEPARATOR)
virtual_column(ca_sym, :type => :string, :uses => :custom_attributes)

where_args = {}
where_args[:name] = custom_attribute_without_section
define_method(ca_sym) do
return self[sanatized_column_alias] if has_attribute?(sanatized_column_alias)

where_args = {}
where_args[:name] = name_val
where_args[:section] = section if section

custom_attributes.find_by(where_args).try(:value)
end
end

def self.custom_attribute_arel(name_val, section, column_alias)
ca_field = CustomAttribute.arel_table

field_where = ca_field[:resource_id].eq(arel_table[:id])
field_where = field_where.and(ca_field[:resource_type].eq(base_class.name))
field_where = field_where.and(ca_field[:name].eq(name_val))
field_where = field_where.and(ca_field[:section].eq(section)) if section

# Because there is a `find_by` in the `define_method` above, we are
# using a `take(1)` here as well, since a limit is assumed in each.
# Without it, there can be some invalid queries if more than one result
# is returned.
ca_field.project(:value).where(field_where).take(1).as(column_alias)
end
end

def self.to_human(column)
Expand Down

0 comments on commit cb5a8cb

Please sign in to comment.