| refactored ReportCache.process - reportable - Fork of reportable required by Wa… | |
| Log | |
| Files | |
| Refs | |
| README | |
| --- | |
| commit dea59962791f1628977ef04d95787cede7bd5fab | |
| parent b3b59ff8b4f366dac401283d3f80631b8d71d4bf | |
| Author: Marco Otte-Witte <[email protected]> | |
| Date: Thu, 15 Jan 2009 12:19:00 +0100 | |
| refactored ReportCache.process | |
| Diffstat: | |
| M lib/kvlr/reports_as_sparkline/repo… | 2 +- | |
| M lib/kvlr/reports_as_sparkline/repo… | 18 +++++++++--------- | |
| M spec/classes/report_cache_spec.rb | 30 +++++++++++++++--------------- | |
| M spec/classes/report_spec.rb | 22 +++++++++++++++++----- | |
| 4 files changed, 42 insertions(+), 30 deletions(-) | |
| --- | |
| diff --git a/lib/kvlr/reports_as_sparkline/report.rb b/lib/kvlr/reports_as_spar… | |
| @@ -46,7 +46,7 @@ module Kvlr #:nodoc: | |
| custom_conditions = options.key?(:conditions) | |
| options.reverse_merge!(@options) | |
| options[:grouping] = Grouping.new(options[:grouping]) unless options[:… | |
| - ReportCache.process(self, options[:limit], options[:grouping], custom_… | |
| + ReportCache.process(self, options, !custom_conditions) do |begin_at| | |
| read_data(begin_at, options[:grouping], options[:conditions]) | |
| end | |
| end | |
| diff --git a/lib/kvlr/reports_as_sparkline/report_cache.rb b/lib/kvlr/reports_a… | |
| @@ -4,42 +4,42 @@ module Kvlr #:nodoc: | |
| class ReportCache < ActiveRecord::Base #:nodoc: | |
| - def self.process(report, limit, grouping, no_cache = false, &block) | |
| + def self.process(report, options, cache = true, &block) | |
| raise ArgumentError.new('A block must be given') unless block_given? | |
| self.transaction do | |
| cached_data = [] | |
| - last_reporting_period_to_read = ReportingPeriod.first(grouping, limi… | |
| - unless no_cache | |
| + last_reporting_period_to_read = ReportingPeriod.first(options[:group… | |
| + if cache | |
| cached_data = self.find( | |
| :all, | |
| :conditions => [ | |
| 'model_name = ? AND report_name = ? AND grouping = ? AND aggre… | |
| report.klass.to_s, | |
| report.name.to_s, | |
| - grouping.identifier.to_s, | |
| + options[:grouping].identifier.to_s, | |
| report.aggregation.to_s, | |
| last_reporting_period_to_read.date_time | |
| ], | |
| - :limit => limit, | |
| + :limit => options[:limit], | |
| :order => 'reporting_period ASC' | |
| ) | |
| - last_reporting_period_to_read = ReportingPeriod.new(grouping, cach… | |
| + last_reporting_period_to_read = ReportingPeriod.new(options[:group… | |
| end | |
| new_data = yield(last_reporting_period_to_read.date_time) | |
| - prepare_result(new_data, cached_data, last_reporting_period_to_read,… | |
| + prepare_result(new_data, cached_data, last_reporting_period_to_read,… | |
| end | |
| end | |
| private | |
| - def self.prepare_result(new_data, cached_data, last_reporting_period_t… | |
| + def self.prepare_result(new_data, cached_data, last_reporting_period_t… | |
| new_data.map! { |data| [ReportingPeriod.from_db_string(grouping, dat… | |
| result = cached_data.map { |cached| [cached.reporting_period, cached… | |
| current_reporting_period = ReportingPeriod.new(grouping) | |
| reporting_period = last_reporting_period_to_read | |
| while reporting_period < current_reporting_period | |
| cached = build_cached_data(report, grouping, reporting_period, fin… | |
| - cached.save! unless no_cache | |
| + cached.save! if cache | |
| result << [reporting_period.date_time, cached.value] | |
| reporting_period = reporting_period.next | |
| end | |
| diff --git a/spec/classes/report_cache_spec.rb b/spec/classes/report_cache_spec… | |
| @@ -15,19 +15,19 @@ describe Kvlr::ReportsAsSparkline::ReportCache do | |
| it 'should raise an ArgumentError if no block is given' do | |
| lambda do | |
| - Kvlr::ReportsAsSparkline::ReportCache.process(@report, 10, @report.opt… | |
| + Kvlr::ReportsAsSparkline::ReportCache.process(@report, { :limit => 10,… | |
| end.should raise_error(ArgumentError) | |
| end | |
| it 'sould start a transaction' do | |
| Kvlr::ReportsAsSparkline::ReportCache.should_receive(:transaction) | |
| - Kvlr::ReportsAsSparkline::ReportCache.process(@report, 10, @report.optio… | |
| + Kvlr::ReportsAsSparkline::ReportCache.process(@report, { :limit => 10, :… | |
| end | |
| it 'should yield to the given block' do | |
| lambda { | |
| - Kvlr::ReportsAsSparkline::ReportCache.process(@report, 10, @report.opt… | |
| + Kvlr::ReportsAsSparkline::ReportCache.process(@report, { :limit => 10,… | |
| }.should raise_error(YieldMatchException) | |
| end | |
| @@ -46,7 +46,7 @@ describe Kvlr::ReportsAsSparkline::ReportCache do | |
| :order => 'reporting_period ASC' | |
| ) | |
| - Kvlr::ReportsAsSparkline::ReportCache.process(@report, 10, @report.optio… | |
| + Kvlr::ReportsAsSparkline::ReportCache.process(@report, { :limit => 10, :… | |
| end | |
| it "should read existing data from the cache for the correct grouping if o… | |
| @@ -65,7 +65,7 @@ describe Kvlr::ReportsAsSparkline::ReportCache do | |
| :order => 'reporting_period ASC' | |
| ) | |
| - Kvlr::ReportsAsSparkline::ReportCache.process(@report, 10, grouping) { [… | |
| + Kvlr::ReportsAsSparkline::ReportCache.process(@report, { :limit => 10, :… | |
| end | |
| it 'should prepare the results before it returns them' do | |
| @@ -73,13 +73,13 @@ describe Kvlr::ReportsAsSparkline::ReportCache do | |
| cached_data = [] | |
| Kvlr::ReportsAsSparkline::ReportCache.stub!(:find).and_return(cached_dat… | |
| last_reporting_period_to_read = Kvlr::ReportsAsSparkline::ReportingPerio… | |
| - Kvlr::ReportsAsSparkline::ReportCache.should_receive(:prepare_result).on… | |
| + Kvlr::ReportsAsSparkline::ReportCache.should_receive(:prepare_result).on… | |
| - Kvlr::ReportsAsSparkline::ReportCache.process(@report, 10, @report.optio… | |
| + Kvlr::ReportsAsSparkline::ReportCache.process(@report, { :limit => 10, :… | |
| end | |
| it 'should yield the first reporting period if the cache is empty' do | |
| - Kvlr::ReportsAsSparkline::ReportCache.process(@report, 10, @report.optio… | |
| + Kvlr::ReportsAsSparkline::ReportCache.process(@report, { :limit => 10, :… | |
| begin_at.should == Kvlr::ReportsAsSparkline::ReportingPeriod.first(@re… | |
| [] | |
| end | |
| @@ -97,22 +97,22 @@ describe Kvlr::ReportsAsSparkline::ReportCache do | |
| }) | |
| Kvlr::ReportsAsSparkline::ReportCache.stub!(:find).and_return([cached]) | |
| - Kvlr::ReportsAsSparkline::ReportCache.process(@report, 10, @report.optio… | |
| + Kvlr::ReportsAsSparkline::ReportCache.process(@report, { :limit => 10, :… | |
| begin_at.should == reporting_period.next.date_time | |
| [] | |
| end | |
| end | |
| - describe 'with no_cache = true' do | |
| + describe 'with cache = false' do | |
| it 'should not read any data from cache' do | |
| Kvlr::ReportsAsSparkline::ReportCache.should_not_receive(:find) | |
| - Kvlr::ReportsAsSparkline::ReportCache.process(@report, 10, @report.opt… | |
| + Kvlr::ReportsAsSparkline::ReportCache.process(@report, { :limit => 10,… | |
| end | |
| it 'should yield the first reporting period' do | |
| - Kvlr::ReportsAsSparkline::ReportCache.process(@report, 10, @report.opt… | |
| + Kvlr::ReportsAsSparkline::ReportCache.process(@report, { :limit => 10,… | |
| begin_at.should == Kvlr::ReportsAsSparkline::ReportingPeriod.first(@… | |
| [] | |
| end | |
| @@ -182,19 +182,19 @@ describe Kvlr::ReportsAsSparkline::ReportCache do | |
| result[0][1].should be_kind_of(Float) | |
| end | |
| - describe 'with no_cache = true' do | |
| + describe 'with cache = false' do | |
| it 'should not save the created Kvlr::ReportsAsSparkline::ReportCache' do | |
| @cached.should_not_receive(:save!) | |
| - Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, @new_data,… | |
| + Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, @new_data,… | |
| end | |
| it 'should not update the last cached record if new data has been read f… | |
| Kvlr::ReportsAsSparkline::ReportingPeriod.stub!(:from_db_string).and_r… | |
| @cached.should_not_receive(:update_attributes!) | |
| - Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, @new_data,… | |
| + Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, @new_data,… | |
| end | |
| end | |
| diff --git a/spec/classes/report_spec.rb b/spec/classes/report_spec.rb | |
| @@ -9,13 +9,21 @@ describe Kvlr::ReportsAsSparkline::Report do | |
| describe '#run' do | |
| it 'should process the data with the report cache' do | |
| - Kvlr::ReportsAsSparkline::ReportCache.should_receive(:process).once.with… | |
| + Kvlr::ReportsAsSparkline::ReportCache.should_receive(:process).once.with( | |
| + @report, | |
| + { :limit => 100, :grouping => @report.options[:grouping], :conditions … | |
| + true | |
| + ) | |
| @report.run | |
| end | |
| - it 'should process the data with the report cache and specify no_cache whe… | |
| - Kvlr::ReportsAsSparkline::ReportCache.should_receive(:process).once.with… | |
| + it 'should process the data with the report cache and specify cache = fals… | |
| + Kvlr::ReportsAsSparkline::ReportCache.should_receive(:process).once.with( | |
| + @report, | |
| + { :limit => 100, :grouping => @report.options[:grouping], :conditions … | |
| + false | |
| + ) | |
| @report.run(:conditions => { :some => :condition }) | |
| end | |
| @@ -29,9 +37,13 @@ describe Kvlr::ReportsAsSparkline::Report do | |
| it 'should use a custom grouping if one is specified' do | |
| grouping = Kvlr::ReportsAsSparkline::Grouping.new(:month) | |
| Kvlr::ReportsAsSparkline::Grouping.should_receive(:new).once.with(:month… | |
| - Kvlr::ReportsAsSparkline::ReportCache.should_receive(:process).once.with… | |
| + Kvlr::ReportsAsSparkline::ReportCache.should_receive(:process).once.with( | |
| + @report, | |
| + { :limit => 100, :grouping => grouping, :conditions => [] }, | |
| + true | |
| + ) | |
| - @report.run(:conditions => { :some => :condition }, :grouping => :month) | |
| + @report.run(:grouping => :month) | |
| end | |
| for grouping in [:hour, :day, :week, :month] do |