| refactored and added specs for ReportCache::prepare_result - reportable - Fork … | |
| Log | |
| Files | |
| Refs | |
| README | |
| --- | |
| commit ca91ca842c410fa1afea209b550ac8ed2d812e35 | |
| parent f7accf08304988c6a9ed3ad7155e887a15054257 | |
| Author: marcoow <[email protected]> | |
| Date: Thu, 11 Dec 2008 01:28:36 +0800 | |
| refactored and added specs for ReportCache::prepare_result | |
| Signed-off-by: Marco Otte-Witte <[email protected]> | |
| Diffstat: | |
| M lib/kvlr/reports_as_sparkline/cumu… | 3 ++- | |
| M lib/kvlr/reports_as_sparkline/repo… | 1 + | |
| M lib/kvlr/reports_as_sparkline/repo… | 79 +++++++++++++--------------… | |
| M spec/boot.rb | 2 +- | |
| M spec/other/cumulated_report_spec.rb | 36 ++++++++++++++++++++++++++---… | |
| M spec/other/report_cache_spec.rb | 112 ++++++++++++++++++++++++++---… | |
| M spec/other/report_spec.rb | 4 ++++ | |
| 7 files changed, 169 insertions(+), 68 deletions(-) | |
| --- | |
| diff --git a/lib/kvlr/reports_as_sparkline/cumulated_report.rb b/lib/kvlr/repor… | |
| @@ -17,7 +17,8 @@ module Kvlr #:nodoc: | |
| acc += element[1].to_f | |
| result << [element[0], acc] | |
| end | |
| - result.reverse | |
| + result.reverse! | |
| + result | |
| end | |
| end | |
| diff --git a/lib/kvlr/reports_as_sparkline/report.rb b/lib/kvlr/reports_as_spar… | |
| @@ -61,6 +61,7 @@ module Kvlr #:nodoc: | |
| end | |
| raise ArgumentError.new("Invalid aggregation #{options[:aggregat… | |
| raise ArgumentError.new("Invalid grouping #{options[:grouping]}"… | |
| + raise ArgumentError.new('The name of the column holding the valu… | |
| when :run | |
| options.each_key do |k| | |
| raise ArgumentError.new("Invalid option #{k}") unless [:limit,… | |
| diff --git a/lib/kvlr/reports_as_sparkline/report_cache.rb b/lib/kvlr/reports_a… | |
| @@ -9,62 +9,51 @@ module Kvlr #:nodoc: | |
| def self.process(report, limit, no_cache = false, &block) | |
| raise ArgumentError.new('A block must be given') unless block_given? | |
| self.transaction do | |
| - cached_data = if no_cache | |
| - [] | |
| - else | |
| - self.find( | |
| - :all, | |
| - :conditions => { | |
| - :model_name => report.klass.to_s, | |
| - :report_name => report.name.to_s, | |
| - :grouping => report.grouping.identifier.to_s, | |
| - :aggregation => report.aggregation.to_s | |
| - }, | |
| - :limit => limit, | |
| - :order => 'reporting_period DESC' | |
| - ) | |
| - end | |
| - last_reporting_period_to_read = if cached_data.empty? | |
| - ReportingPeriod.first(report.grouping, limit) | |
| - else | |
| - cached_data.last.reporting_period | |
| - end | |
| + cached_data = [] | |
| + last_reporting_period_to_read = ReportingPeriod.first(report.groupin… | |
| + unless no_cache | |
| + cached_data = self.find( | |
| + :all, | |
| + :conditions => { | |
| + :model_name => report.klass.to_s, | |
| + :report_name => report.name.to_s, | |
| + :grouping => report.grouping.identifier.to_s, | |
| + :aggregation => report.aggregation.to_s | |
| + }, | |
| + :limit => limit, | |
| + :order => 'reporting_period DESC' | |
| + ) | |
| + last_reporting_period_to_read = cached_data.last.reporting_period … | |
| + end | |
| new_data = yield(last_reporting_period_to_read.date_time) | |
| - update_cache(new_data, cached_data, last_reporting_period_to_read, r… | |
| + prepare_result(new_data, cached_data, last_reporting_period_to_read,… | |
| end | |
| end | |
| private | |
| - def self.update_cache(new_data, cached_data, last_reporting_period_to_… | |
| + def self.prepare_result(new_data, cached_data, last_reporting_period_t… | |
| new_data.map! { |data| [ReportingPeriod.from_db_string(report.groupi… | |
| - current_reporting_period = ReportingPeriod.new(report.grouping) | |
| - reporting_period = current_reporting_period | |
| + reporting_period = ReportingPeriod.new(report.grouping) | |
| result = [] | |
| begin | |
| data = new_data.detect { |data| data[0] == reporting_period } | |
| - if data && cached = cached_data.detect { |cached| cached.reporting… | |
| - if no_cache | |
| - cached.update_attributes!(:value => data[1]) | |
| - else | |
| - cached.value = data[1] | |
| - end | |
| - else | |
| - data = self.new( | |
| - :model_name => report.klass.to_s, | |
| - :report_name => report.name.to_s, | |
| - :grouping => report.grouping.identifier.to_s, | |
| - :aggregation => report.aggregation.to_s, | |
| - :reporting_period => reporting_period, | |
| - :value => (data ? data[1] : 0) | |
| - ) | |
| - data.save! unless no_cache | |
| - data = [data.reporting_period, data.value] | |
| - end | |
| - result << data | |
| + cached = self.new( | |
| + :model_name => report.klass.to_s, | |
| + :report_name => report.name.to_s, | |
| + :grouping => report.grouping.identifier.to_s, | |
| + :aggregation => report.aggregation.to_s, | |
| + :reporting_period => reporting_period, | |
| + :value => (data ? data[1] : 0) | |
| + ) | |
| + cached.save! unless no_cache | |
| + result << [cached.reporting_period.date_time, cached.value] | |
| reporting_period = reporting_period.previous | |
| - end while (reporting_period != last_reporting_period_to_read) | |
| - result.map { |r| [r[0].date_time, r[1]] } | |
| + end while reporting_period != last_reporting_period_to_read | |
| + if !no_cache && cached = cached_data.detect { |cached| cached.report… | |
| + cached.update_attributes!(:value => data[1]) | |
| + end | |
| + result | |
| end | |
| end | |
| diff --git a/spec/boot.rb b/spec/boot.rb | |
| @@ -19,5 +19,5 @@ FileUtils.mkdir_p File.join(File.dirname(__FILE__), 'log') | |
| ActiveRecord::Base.logger = Logger.new(File.join(File.dirname(__FILE__), 'log'… | |
| databases = YAML::load(IO.read(File.join(File.dirname(__FILE__), 'db', 'databa… | |
| -ActiveRecord::Base.establish_connection(databases['postgresql']) | |
| +ActiveRecord::Base.establish_connection(databases['sqlite3']) | |
| load(File.join(File.dirname(__FILE__), 'db', 'schema.rb')) | |
| diff --git a/spec/other/cumulated_report_spec.rb b/spec/other/cumulated_report_… | |
| @@ -14,6 +14,12 @@ describe Kvlr::ReportsAsSparkline::CumulatedReport do | |
| @report.run | |
| end | |
| + it 'should return an array of the same length as the specified limit' do | |
| + @report = Kvlr::ReportsAsSparkline::CumulatedReport.new(User, :cumulated… | |
| + | |
| + @report.run.length.should == 10 | |
| + end | |
| + | |
| for grouping in [:hour, :day, :week, :month] do | |
| describe "for grouping #{grouping.to_s}" do | |
| @@ -24,6 +30,24 @@ describe Kvlr::ReportsAsSparkline::CumulatedReport do | |
| User.create!(:login => 'test 3', :created_at => Time.now - 3.send(gr… | |
| end | |
| + describe do | |
| + | |
| + before do | |
| + @grouping = Kvlr::ReportsAsSparkline::Grouping.new(grouping) | |
| + @report = Kvlr::ReportsAsSparkline::CumulatedReport.new(User, :cum… | |
| + @result = @report.run | |
| + end | |
| + | |
| + it "should return data starting with the current reporting period" do | |
| + @result.first[0].should == Kvlr::ReportsAsSparkline::ReportingPeri… | |
| + end | |
| + | |
| + it "should return data ending with reporting period (Time.now - (lim… | |
| + @result.last[0].should == Kvlr::ReportsAsSparkline::ReportingPerio… | |
| + end | |
| + | |
| + end | |
| + | |
| it 'should return correct data for :aggregation => :count' do | |
| @report = Kvlr::ReportsAsSparkline::CumulatedReport.new(User, :regis… | |
| result = @report.run | |
| @@ -36,7 +60,7 @@ describe Kvlr::ReportsAsSparkline::CumulatedReport do | |
| it 'should return correct data for :aggregation => :sum' do | |
| @report = Kvlr::ReportsAsSparkline::CumulatedReport.new(User, :regis… | |
| - result = @report.run().to_a | |
| + result = @report.run() | |
| result[0][1].should == 6 | |
| result[1][1].should == 6 | |
| @@ -46,7 +70,7 @@ describe Kvlr::ReportsAsSparkline::CumulatedReport do | |
| it 'should return correct data with custom conditions for :aggregation… | |
| @report = Kvlr::ReportsAsSparkline::CumulatedReport.new(User, :regis… | |
| - result = @report.run(:conditions => ['login IN (?)', ['test 1', 'tes… | |
| + result = @report.run(:conditions => ['login IN (?)', ['test 1', 'tes… | |
| result[0][1].should == 2 | |
| result[1][1].should == 2 | |
| @@ -56,7 +80,7 @@ describe Kvlr::ReportsAsSparkline::CumulatedReport do | |
| it 'should return correct data with custom conditions for :aggregation… | |
| @report = Kvlr::ReportsAsSparkline::CumulatedReport.new(User, :regis… | |
| - result = @report.run(:conditions => ['login IN (?)', ['test 1', 'tes… | |
| + result = @report.run(:conditions => ['login IN (?)', ['test 1', 'tes… | |
| result[0][1].should == 3 | |
| result[1][1].should == 3 | |
| @@ -68,10 +92,10 @@ describe Kvlr::ReportsAsSparkline::CumulatedReport do | |
| User.destroy_all | |
| end | |
| - after(:each) do | |
| - Kvlr::ReportsAsSparkline::ReportCache.destroy_all | |
| - end | |
| + end | |
| + after(:each) do | |
| + Kvlr::ReportsAsSparkline::ReportCache.destroy_all | |
| end | |
| end | |
| diff --git a/spec/other/report_cache_spec.rb b/spec/other/report_cache_spec.rb | |
| @@ -2,33 +2,36 @@ require File.join(File.dirname(__FILE__), '..', 'spec_helper') | |
| describe Kvlr::ReportsAsSparkline::ReportCache do | |
| + before do | |
| + @report = Kvlr::ReportsAsSparkline::Report.new(User, :registrations) | |
| + end | |
| + | |
| describe '#process' do | |
| before do | |
| - @report = Kvlr::ReportsAsSparkline::Report.new(User, :registrations) | |
| Kvlr::ReportsAsSparkline::ReportCache.stub!(:find).and_return([]) | |
| - Kvlr::ReportsAsSparkline::ReportCache.stub!(:update_cache).and_return([]) | |
| + Kvlr::ReportsAsSparkline::ReportCache.stub!(:prepare_result).and_return(… | |
| end | |
| it 'should raise an ArgumentError if no block is given' do | |
| lambda do | |
| - Kvlr::ReportsAsSparkline::ReportCache.process(@report, 100) | |
| + Kvlr::ReportsAsSparkline::ReportCache.process(@report, 10) | |
| end.should raise_error(ArgumentError) | |
| end | |
| it 'sould start a transaction' do | |
| Kvlr::ReportsAsSparkline::ReportCache.should_receive(:transaction) | |
| - Kvlr::ReportsAsSparkline::ReportCache.process(@report, 100) {} | |
| + Kvlr::ReportsAsSparkline::ReportCache.process(@report, 10) {} | |
| end | |
| it 'should yield to the given block' do | |
| lambda { | |
| - Kvlr::ReportsAsSparkline::ReportCache.process(@report, 100) { raise Yi… | |
| + Kvlr::ReportsAsSparkline::ReportCache.process(@report, 10) { raise Yie… | |
| }.should raise_error(YieldMatchException) | |
| end | |
| - it 'should read existing data for the report from cache' do | |
| + it 'should read existing data for the report from cache if no_cache' do | |
| Kvlr::ReportsAsSparkline::ReportCache.should_receive(:find).once.with( | |
| :all, | |
| :conditions => { | |
| @@ -37,22 +40,22 @@ describe Kvlr::ReportsAsSparkline::ReportCache do | |
| :grouping => @report.grouping.identifier.to_s, | |
| :aggregation => @report.aggregation.to_s | |
| }, | |
| - :limit => 100, | |
| + :limit => 10, | |
| :order => "reporting_period DESC" | |
| ) | |
| - Kvlr::ReportsAsSparkline::ReportCache.process(@report, 100) { [] } | |
| + Kvlr::ReportsAsSparkline::ReportCache.process(@report, 10) { [] } | |
| end | |
| it 'should update the cache' do | |
| - Kvlr::ReportsAsSparkline::ReportCache.should_receive(:update_cache) | |
| + Kvlr::ReportsAsSparkline::ReportCache.should_receive(:prepare_result) | |
| - Kvlr::ReportsAsSparkline::ReportCache.process(@report, 100) { [] } | |
| + Kvlr::ReportsAsSparkline::ReportCache.process(@report, 10) { [] } | |
| end | |
| it 'should yield the first reporting period if the cache is empty' do | |
| - Kvlr::ReportsAsSparkline::ReportCache.process(@report, 100) do |begin_at| | |
| - begin_at.should == Kvlr::ReportsAsSparkline::ReportingPeriod.first(@re… | |
| + Kvlr::ReportsAsSparkline::ReportCache.process(@report, 10) do |begin_at| | |
| + begin_at.should == Kvlr::ReportsAsSparkline::ReportingPeriod.first(@re… | |
| [] | |
| end | |
| end | |
| @@ -69,7 +72,7 @@ describe Kvlr::ReportsAsSparkline::ReportCache do | |
| }) | |
| Kvlr::ReportsAsSparkline::ReportCache.stub!(:find).and_return([cached]) | |
| - Kvlr::ReportsAsSparkline::ReportCache.process(@report, 100) do |begin_at| | |
| + Kvlr::ReportsAsSparkline::ReportCache.process(@report, 10) do |begin_at| | |
| begin_at.should == reporting_period.date_time | |
| [] | |
| end | |
| @@ -80,14 +83,93 @@ describe Kvlr::ReportsAsSparkline::ReportCache do | |
| it 'should not read any data from cache' do | |
| Kvlr::ReportsAsSparkline::ReportCache.should_not_receive(:find) | |
| - Kvlr::ReportsAsSparkline::ReportCache.process(@report, 100, true) {} | |
| + Kvlr::ReportsAsSparkline::ReportCache.process(@report, 10, true) {} | |
| + end | |
| + | |
| + it 'should yield the first reporting period' do | |
| + Kvlr::ReportsAsSparkline::ReportCache.process(@report, 10, true) do |b… | |
| + begin_at.should == Kvlr::ReportsAsSparkline::ReportingPeriod.first(@… | |
| + [] | |
| + end | |
| end | |
| end | |
| end | |
| - describe '#update_cache' do | |
| + describe '#prepare_result' do | |
| + | |
| + before do | |
| + @last_reporting_period_to_read = Kvlr::ReportsAsSparkline::ReportingPeri… | |
| + @new_data = [['2008/12', 1.0]] | |
| + Kvlr::ReportsAsSparkline::ReportingPeriod.stub!(:from_db_string).and_ret… | |
| + @cached = Kvlr::ReportsAsSparkline::ReportCache.new | |
| + @cached.stub!(:save!) | |
| + @cached.stub!(:reporting_period).and_return(Kvlr::ReportsAsSparkline::Re… | |
| + Kvlr::ReportsAsSparkline::ReportCache.stub!(:new).and_return(@cached) | |
| + end | |
| + | |
| + it 'should convert the date strings from the newly read data to reporting … | |
| + Kvlr::ReportsAsSparkline::ReportingPeriod.should_receive(:from_db_string… | |
| + | |
| + Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, @new_data, [… | |
| + end | |
| + | |
| + it 'should create a new Kvlr::ReportsAsSparkline::ReportCache with the cor… | |
| + Kvlr::ReportsAsSparkline::ReportCache.should_receive(:new).once.with( | |
| + :model_name => @report.klass.to_s, | |
| + :report_name => @report.name.to_s, | |
| + :grouping => @report.grouping.identifier.to_s, | |
| + :aggregation => @report.aggregation.to_s, | |
| + :reporting_period => anything(), | |
| + :value => 0 | |
| + ).and_return(@cached) | |
| + | |
| + Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, [], [], @las… | |
| + end | |
| + | |
| + it 'should create a new Kvlr::ReportsAsSparkline::ReportCache with the cor… | |
| + Kvlr::ReportsAsSparkline::ReportCache.should_receive(:new).once.with( | |
| + :model_name => @report.klass.to_s, | |
| + :report_name => @report.name.to_s, | |
| + :grouping => @report.grouping.identifier.to_s, | |
| + :aggregation => @report.aggregation.to_s, | |
| + :reporting_period => anything(), | |
| + :value => 1.0 | |
| + ).and_return(@cached) | |
| + | |
| + Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, @new_data, [… | |
| + end | |
| + | |
| + it 'should save the created Kvlr::ReportsAsSparkline::ReportCache if no_ca… | |
| + @cached.should_receive(:save!).once | |
| + | |
| + Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, @new_data, [… | |
| + end | |
| + | |
| + it 'should not save the created Kvlr::ReportsAsSparkline::ReportCache if n… | |
| + @cached.should_not_receive(:save!) | |
| + | |
| + Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, @new_data, [… | |
| + end | |
| + | |
| + it 'should return an array of arrays of Dates and Floats' do | |
| + result = Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, @ne… | |
| + | |
| + result.should be_kind_of(Array) | |
| + result[0].should be_kind_of(Array) | |
| + result[0][0].should be_kind_of(Date) | |
| + result[0][1].should be_kind_of(Float) | |
| + end | |
| + | |
| + it 'should return an array with :limit elements' do | |
| + result = Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, @ne… | |
| + | |
| + result.length.should == 10 | |
| + end | |
| + | |
| + #TODO: add specs for update of record in cache fpr last_reporting_period_t… | |
| + | |
| end | |
| end | |
| diff --git a/spec/other/report_spec.rb b/spec/other/report_spec.rb | |
| @@ -195,6 +195,10 @@ describe Kvlr::ReportsAsSparkline::Report do | |
| lambda { @report.send(:ensure_valid_options, { :grouping => :decade })… | |
| end | |
| + it 'should raise an error if aggregation :sum is spesicied without the n… | |
| + lambda { @report.send(:ensure_valid_options, { :aggregation => :sum })… | |
| + end | |
| + | |
| end | |
| describe 'for context :run' do |