Skip to content

Commit

Permalink
Merge pull request #16683 from lpichler/stop_cash_fields_for_chargeba…
Browse files Browse the repository at this point in the history
…ckvm_report

Stop cashing fields for ChargebackVm report
(cherry picked from commit 6352baf)

https://bugzilla.redhat.com/show_bug.cgi?id=1517956
  • Loading branch information
gtanzillo authored and simaishi committed Jan 3, 2018
1 parent 535dc39 commit cf02454
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 0 deletions.
16 changes: 16 additions & 0 deletions app/models/chargeback_vm.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,22 @@ class ChargebackVm < Chargeback
:total_cost => :float,
)

DEFAULT_STORAGE_METRICS = %w(
storage_allocated_unclassified_metric
storage_allocated_unclassified_cost
storage_allocated_metric
storage_allocated_cost
).freeze

def self.attribute_names
loaded_attribute_names = super
loaded_storage_allocated_attributes = loaded_attribute_names.select { |x| x.starts_with?('storage_allocated_') }
loaded_sub_metric_fields = loaded_storage_allocated_attributes - DEFAULT_STORAGE_METRICS
non_existing_sub_metric_fields = loaded_sub_metric_fields - dynamic_columns_for(:float).keys

loaded_attribute_names - non_existing_sub_metric_fields
end

# example:
# dynamic_columns_for(:group => [:total])
# returns:
Expand Down
2 changes: 2 additions & 0 deletions lib/miq_expression.rb
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,7 @@ def self.model_details(model, opts = {:typ => "all", :include_model => true, :in
unless opts[:typ] == "count" || opts[:typ] == "find"
@column_cache ||= {}
key = "#{model}_#{opts[:interval]}_#{opts[:include_model] || false}"
@column_cache[key] = nil if model == "ChargebackVm"
@column_cache[key] ||= get_column_details(relats[:columns], model, model, opts).sort! { |a, b| a.to_s <=> b.to_s }
result.concat(@column_cache[key])

Expand Down Expand Up @@ -901,6 +902,7 @@ def self.tag_details(path, opts)

def self.get_relats(model)
@model_relats ||= {}
@model_relats[model] = nil if model == "ChargebackVm"
@model_relats[model] ||= build_relats(model)
end

Expand Down
51 changes: 51 additions & 0 deletions spec/lib/miq_expression_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,57 @@
end
expect(displayed_columms).to match_array(expected_columns)
end

context 'with ChargebackVm' do
context 'with dynamic fields' do
let(:volume_1) { FactoryGirl.create(:cloud_volume, :volume_type => 'TYPE1') }
let(:volume_2) { FactoryGirl.create(:cloud_volume, :volume_type => 'TYPE2') }
let(:volume_3) { FactoryGirl.create(:cloud_volume, :volume_type => 'TYPE3') }
let(:model) { "ChargebackVm" }
let(:volume_1_type_field_cost) { "#{model}-storage_allocated_#{volume_1.volume_type}_cost" }
let(:volume_2_type_field_cost) { "#{model}-storage_allocated_#{volume_2.volume_type}_cost" }
let(:volume_3_type_field_cost) { "#{model}-storage_allocated_#{volume_3.volume_type}_cost" }

before(:each) do
volume_1
volume_2
end

it 'returns uncached actual fields also when dynamic fields chas been changed' do
report_fields = described_class.reporting_available_fields(model).map(&:second)

expect(report_fields).to include(volume_1_type_field_cost)
expect(report_fields).to include(volume_2_type_field_cost)

# case: change name
volume_2.update_attributes!(:volume_type => 'NEW_TYPE_2')
report_fields = described_class.reporting_available_fields(model).map(&:second)
expect(report_fields).to include(volume_1_type_field_cost)
expect(report_fields).not_to include(volume_2_type_field_cost) # old field

# check existence of new name
report_fields = described_class.reporting_available_fields(model).map(&:second)
volume_2_type_field_cost = "#{model}-storage_allocated_#{volume_2.volume_type}_cost"
expect(report_fields).to include(volume_1_type_field_cost)
expect(report_fields).to include(volume_2_type_field_cost)

# case: add volume_type
volume_3
report_fields = described_class.reporting_available_fields(model).map(&:second)
expect(report_fields).to include(volume_1_type_field_cost)
expect(report_fields).to include(volume_3_type_field_cost)

# case: remove volume_types
volume_2.destroy
volume_3.destroy

report_fields = described_class.reporting_available_fields(model).map(&:second)
expect(report_fields).to include(volume_1_type_field_cost)
expect(report_fields).not_to include(volume_2_type_field_cost)
expect(report_fields).not_to include(volume_3_type_field_cost)
end
end
end
end

describe "#valid?" do
Expand Down
14 changes: 14 additions & 0 deletions spec/models/chargeback_vm_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,20 @@
expect(subject.storage_allocated_hdd_metric).to eq(1.gigabytes)
expect(subject.storage_allocated_hdd_cost).to eq(state_data[:allocated_disk_types]['hdd'] / 1.gigabytes * count_hourly_rate * hours_in_day)
end

it "doesn't return removed cloud volume types fields" do
described_class.refresh_dynamic_metric_columns

fields = described_class.attribute_names
cloud_volume_hdd_field = "storage_allocated_#{cloud_volume_hdd.volume_type}_metric"
expect(fields).to include(cloud_volume_hdd_field)

cloud_volume_hdd.destroy

described_class.refresh_dynamic_metric_columns
fields = described_class.attribute_names
expect(fields).not_to include(cloud_volume_hdd_field)
end
end

subject { ChargebackVm.build_results_for_report_ChargebackVm(options).first.first }
Expand Down

0 comments on commit cf02454

Please sign in to comment.