Merge pull request #13331 from isimluk/fix-rate-adjustment-rounding-bug

Chargeback: Fix rate adjustment rounding bug

app/models/chargeback_rate_detail.rb

@@ -57,10 +57,7 @@ def fixed?
   def find_rate(value)
     fixed_rate = 0.0
     variable_rate = 0.0
-    tier_found, = chargeback_tiers.select do |tier|
-      tier.starts_with_zero? && value.zero? ||
-      value > rate_adjustment(tier.start) && value <= rate_adjustment(tier.finish)
-    end
+    tier_found = chargeback_tiers.detect { |tier| tier.includes?(value / rate_adjustment) }
     unless tier_found.nil?
       fixed_rate = tier_found.fixed_rate
       variable_rate = tier_found.variable_rate
@@ -86,7 +83,7 @@ def hourly_cost(value, consumption)
     hourly_fixed_rate    = hourly(fixed_rate, consumption)
     hourly_variable_rate = hourly(variable_rate, consumption)
 
-    hourly_fixed_rate + rate_adjustment(hourly_variable_rate) * value
+    hourly_fixed_rate + rate_adjustment * value * hourly_variable_rate
   end
 
   def hourly(rate, consumption)
@@ -102,41 +99,23 @@ def hourly(rate, consumption)
     hourly_rate
   end
 
-  # Scale the rate in the unit difine by user to the default unit of the metric
-  # It showing the default units of the metrics:
-  # cpu_usagemhz_rate_average --> megahertz
-  # derived_memory_used --> megabytes
-  # derived_memory_available -->megabytes
-  # net_usage_rate_average --> kbps
-  # disk_usage_rate_average --> kbps
-  # derived_vm_allocated_disk_storage --> bytes
-  # derived_vm_used_disk_storage --> bytes
-
-  def rate_adjustment(hr)
-    case metric
-    when "cpu_usagemhz_rate_average" then
-      per_unit == 'megahertz' ? hr : hr = adjustment_measure(hr, 'megahertz')
-    when "derived_memory_used", "derived_memory_available" then
-      per_unit == 'megabytes' ? hr : hr = adjustment_measure(hr, 'megabytes')
-    when "net_usage_rate_average", "disk_usage_rate_average" then
-      per_unit == 'kbps' ? hr : hr = adjustment_measure(hr, 'kbps')
-    when "derived_vm_allocated_disk_storage", "derived_vm_used_disk_storage" then
-      per_unit == 'bytes' ? hr : hr = adjustment_measure(hr, 'bytes')
-    else hr
-    end
-  end
+  # Scale the rate in the unit defined by user -> to the default unit of the metric
+  METRIC_UNITS = {
+    'cpu_usagemhz_rate_average'         => 'megahertz',
+    'derived_memory_used'               => 'megabytes',
+    'derived_memory_available'          => 'megabytes',
+    'net_usage_rate_average'            => 'kbps',
+    'disk_usage_rate_average'           => 'kbps',
+    'derived_vm_allocated_disk_storage' => 'bytes',
+    'derived_vm_used_disk_storage'      => 'bytes'
+  }.freeze
 
-  # Adjusts the hourly rate to the per unit by default
-  def adjustment_measure(hr, pu_destiny)
-    measure = detail_measure
-    pos_pu_destiny = measure.units.index(pu_destiny)
-    pos_per_unit = measure.units.index(per_unit)
-    jumps = (pos_per_unit - pos_pu_destiny).abs
-    if pos_per_unit > pos_pu_destiny
-      hr.to_f / (measure.step**jumps)
-    else
-      hr * (measure.step**jumps)
-    end
+  def rate_adjustment
+    @rate_adjustment ||= if METRIC_UNITS[metric]
+                           detail_measure.adjust(per_unit, METRIC_UNITS[metric])
+                         else
+                           1
+                         end
   end
 
   def affects_report_fields(report_cols)

app/models/chargeback_rate_detail_measure.rb

@@ -13,6 +13,12 @@ def measures
     Hash[units_display.zip(units)]
   end
 
+  def adjust(from_unit, to_unit)
+    return 1 if from_unit == to_unit
+    jumps = units.index(to_unit) - units.index(from_unit)
+    BigDecimal.new(step)**jumps
+  end
+
   private def units_same_length
     unless (units.count == units_display.count)
       errors.add("Units Problem", "Units_display length diferent that the units length")

app/models/chargeback_tier.rb

@@ -17,6 +17,10 @@ def self.to_float(s)
     end
   end
 
+  def includes?(value)
+    starts_with_zero? && value.zero? || value > start && value.to_f <= finish
+  end
+
   def starts_with_zero?
     start.zero?
   end

spec/models/chargeback_rate_detail_spec.rb

@@ -109,7 +109,7 @@
     ].each_slice(2) do |per_unit, rate_adjustment|
       cbd = FactoryGirl.build(:chargeback_rate_detail, :per_unit => per_unit, :metric => 'derived_memory_available',
        :chargeback_rate_detail_measure_id => cbdm.id)
-      expect(cbd.rate_adjustment(value)).to eq(rate_adjustment)
+      expect(cbd.rate_adjustment * value).to eq(rate_adjustment)
     end
   end
 

spec/models/chargeback_vm_spec.rb

@@ -92,7 +92,7 @@
       @service << @vm1
       @service.save
 
-      @vm2 = FactoryGirl.create(:vm_vmware, :name => "test_vm 2", :evm_owner => admin)
+      @vm2 = FactoryGirl.create(:vm_vmware, :name => "test_vm 2", :evm_owner => admin, :created_on => month_beginning)
 
       Range.new(month_beginning, month_end, true).step_value(12.hours).each do |time|
         [@vm1, @vm2].each do |vm|
@@ -233,7 +233,8 @@
     before do
       @tenant = FactoryGirl.create(:tenant)
       @tenant_child = FactoryGirl.create(:tenant, :ancestry => @tenant.id)
-      @vm_tenant = FactoryGirl.create(:vm_vmware, :tenant_id => @tenant_child.id, :name => "test_vm_tenant")
+      @vm_tenant = FactoryGirl.create(:vm_vmware, :tenant_id => @tenant_child.id,
+                                      :name => "test_vm_tenant", :created_on => month_beginning)
 
       Range.new(start_time, finish_time, true).step_value(1.hour).each do |t|
         @vm_tenant.metric_rollups <<