From cf02454b2433b066ca562d5307770f7cc4579eff Mon Sep 17 00:00:00 2001 From: Gregg Tanzillo Date: Tue, 19 Dec 2017 10:08:21 -0500 Subject: [PATCH] Merge pull request #16683 from lpichler/stop_cash_fields_for_chargebackvm_report Stop cashing fields for ChargebackVm report (cherry picked from commit 6352baf2f24699519c7d96c9dc7b96a9f245f1e9) https://bugzilla.redhat.com/show_bug.cgi?id=1517956 --- app/models/chargeback_vm.rb | 16 ++++++++++ lib/miq_expression.rb | 2 ++ spec/lib/miq_expression_spec.rb | 51 +++++++++++++++++++++++++++++++ spec/models/chargeback_vm_spec.rb | 14 +++++++++ 4 files changed, 83 insertions(+) diff --git a/app/models/chargeback_vm.rb b/app/models/chargeback_vm.rb index cefdba24473..d1220c34645 100644 --- a/app/models/chargeback_vm.rb +++ b/app/models/chargeback_vm.rb @@ -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: diff --git a/lib/miq_expression.rb b/lib/miq_expression.rb index 215c075c6eb..aa43f6fa48d 100644 --- a/lib/miq_expression.rb +++ b/lib/miq_expression.rb @@ -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]) @@ -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 diff --git a/spec/lib/miq_expression_spec.rb b/spec/lib/miq_expression_spec.rb index 97cbb21c55d..5f9c7461c37 100644 --- a/spec/lib/miq_expression_spec.rb +++ b/spec/lib/miq_expression_spec.rb @@ -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 diff --git a/spec/models/chargeback_vm_spec.rb b/spec/models/chargeback_vm_spec.rb index 0c3119c35f3..926b20465e9 100644 --- a/spec/models/chargeback_vm_spec.rb +++ b/spec/models/chargeback_vm_spec.rb @@ -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 }