diff --git a/lib/will_paginate/active_record.rb b/lib/will_paginate/active_record.rb index c9a0ae2fe..962e213f4 100644 --- a/lib/will_paginate/active_record.rb +++ b/lib/will_paginate/active_record.rb @@ -47,7 +47,7 @@ def per_page(value = nil) def limit(num) rel = super if rel.current_page - rel.offset((rel.current_page-1) * rel.limit_value) + rel.offset ::WillPaginate.calculate_offset(rel.current_page, rel.limit_value) else rel end @@ -124,10 +124,9 @@ def paginate(options) end def page(num) - pagenum = num.nil? ? 1 : num.to_i - raise ::WillPaginate::InvalidPage, num, pagenum if pagenum < 1 rel = scoped.extending(RelationMethods) - rel = rel.offset((pagenum-1) * (rel.limit_value || per_page)) + pagenum, per_page, offset = ::WillPaginate.process_values(num, rel.limit_value || self.per_page) + rel = rel.offset(offset) rel = rel.limit(per_page) unless rel.limit_value rel.current_page = pagenum rel diff --git a/lib/will_paginate/array.rb b/lib/will_paginate/array.rb index cd7f3ebca..e3dbf6c6c 100644 --- a/lib/will_paginate/array.rb +++ b/lib/will_paginate/array.rb @@ -22,11 +22,11 @@ class Array # McAdam}[http://www.desimcadam.com/archives/8] and later proved to be the # most useful method of will_paginate library. def paginate(options = {}) - raise ArgumentError, "parameter hash expected (got #{options.inspect})" unless Hash === options - - WillPaginate::Collection.create options[:page] || 1, - options[:per_page] || 30, - options[:total_entries] || self.length do |pager| + page = options[:page] || 1 + per_page = options[:per_page] || WillPaginate.per_page + total = options[:total_entries] || self.length + + WillPaginate::Collection.create(page, per_page, total) do |pager| pager.replace self[pager.offset, pager.per_page].to_a end end diff --git a/lib/will_paginate/collection.rb b/lib/will_paginate/collection.rb index 846c6d1a3..4d7c9659b 100644 --- a/lib/will_paginate/collection.rb +++ b/lib/will_paginate/collection.rb @@ -1,27 +1,6 @@ +require 'will_paginate/per_page' + module WillPaginate - # = Invalid page number error - # This is an ArgumentError raised in case a page was requested that is either - # zero or negative number. You should decide how do deal with such errors in - # the controller. - # - # If you're using Rails 2, then this error will automatically get handled like - # 404 Not Found. The hook is in "will_paginate.rb": - # - # ActionController::Base.rescue_responses['WillPaginate::InvalidPage'] = :not_found - # - # If you don't like this, use your preffered method of rescuing exceptions in - # public from your controllers to handle this differently. The +rescue_from+ - # method is a nice addition to Rails 2. - # - # This error is *not* raised when a page further than the last page is - # requested. Use WillPaginate::Collection#out_of_bounds? method to - # check for those cases and manually deal with them as you see fit. - class InvalidPage < ArgumentError - def initialize(page, page_num) #:nodoc: - super "#{page.inspect} given as value, which translates to '#{page_num}' as page number" - end - end - # = The key to pagination # Arrays returned from paginating finds are, in fact, instances of this little # class. You may think of WillPaginate::Collection as an ordinary array with @@ -44,12 +23,9 @@ class Collection < Array # and the total number of entries. The last argument is optional because it # is best to do lazy counting; in other words, count *conditionally* after # populating the collection using the +replace+ method. - def initialize(page, per_page, total = nil) - @current_page = page.to_i - raise InvalidPage.new(page, @current_page) if @current_page < 1 + def initialize(page, per_page = WillPaginate.per_page, total = nil) + @current_page = InvalidPage.validate(page, 'page') @per_page = per_page.to_i - raise ArgumentError, "`per_page` setting cannot be less than 1 (#{@per_page} given)" if @per_page < 1 - self.total_entries = total if total end diff --git a/lib/will_paginate/data_mapper.rb b/lib/will_paginate/data_mapper.rb index 54e3de399..e1586c4f5 100644 --- a/lib/will_paginate/data_mapper.rb +++ b/lib/will_paginate/data_mapper.rb @@ -7,9 +7,8 @@ module WillPaginate module DataMapper module Pagination def page(num) - pagenum = num.nil? ? 1 : num.to_i - raise ::WillPaginate::InvalidPage, num, pagenum if pagenum < 1 - options = {:offset => (pagenum-1) * (query.limit || per_page)} + pagenum, per_page, offset = ::WillPaginate.process_values(num, query.limit || self.per_page) + options = {:offset => offset} options[:limit] = per_page unless query.limit col = new_collection(query.merge(options)) col.current_page = pagenum diff --git a/lib/will_paginate/per_page.rb b/lib/will_paginate/per_page.rb index 29303e1dd..8060f2f47 100644 --- a/lib/will_paginate/per_page.rb +++ b/lib/will_paginate/per_page.rb @@ -24,4 +24,40 @@ def inherited(subclass) # default number of items per page self.per_page = 30 + + # these methods are used internally and are subject to change + module Calculation + def process_values(page, per_page) + page = page.nil? ? 1 : InvalidPage.validate(page, 'page') + per_page = per_page.to_i + offset = calculate_offset(page, per_page) + [page, per_page, offset] + end + + def calculate_offset(page, per_page) + InvalidPage.validate((page - 1) * per_page, 'offset') + end + end + + extend Calculation + + # Raised by paginating methods in case `page` parameter is an invalid number. + # + # In Rails this error is automatically handled as 404 Not Found. + class InvalidPage < ArgumentError + # the maximum value for SQL BIGINT + BIGINT = 9223372036854775807 + + # Returns value cast to integer, raising self if invalid + def self.validate(value, name) + num = value.to_i + rescue NoMethodError + raise self, "#{name} cannot be converted to integer: #{value.inspect}" + else + if 'offset' == name ? (num < 0 or num > BIGINT) : num < 1 + raise self, "invalid #{name}: #{value.inspect}" + end + return num + end + end end diff --git a/spec/collection_spec.rb b/spec/collection_spec.rb index 4d71dc90f..6441e9b0a 100644 --- a/spec/collection_spec.rb +++ b/spec/collection_spec.rb @@ -29,17 +29,6 @@ result.size.should == 30 end - describe "old API" do - it "should fail with numeric params" do - Proc.new { [].paginate(2) }.should raise_error(ArgumentError) - Proc.new { [].paginate(2, 10) }.should raise_error(ArgumentError) - end - - it "should fail with both options and numeric param" do - Proc.new { [].paginate({}, 5) }.should raise_error(ArgumentError) - end - end - it "should give total_entries precedence over actual size" do %w(a b c).paginate(:total_entries => 5).total_entries.should == 5 end @@ -118,26 +107,29 @@ end it "should raise WillPaginate::InvalidPage on invalid input" do - for bad_input in [0, -1, nil, '', 'Schnitzel'] - Proc.new { create bad_input }.should raise_error(WillPaginate::InvalidPage) + [0, -1, nil, '', 'Schnitzel'].each do |bad_input| + lambda { + create bad_input + }.should raise_error(WillPaginate::InvalidPage, "invalid page: #{bad_input.inspect}") end end - it "should raise Argument error on invalid per_page setting" do - Proc.new { create(1, -1) }.should raise_error(ArgumentError) - end - it "should not respond to page_count anymore" do Proc.new { create.page_count }.should raise_error(NoMethodError) end + it "inherits per_page from global value" do + collection = described_class.new(1) + collection.per_page.should == 30 + end + private def create(page = 2, limit = 5, total = nil, &block) if block_given? - WillPaginate::Collection.create(page, limit, total, &block) + described_class.create(page, limit, total, &block) else - WillPaginate::Collection.new(page, limit, total) + described_class.new(page, limit, total) end end diff --git a/spec/finders/active_record_spec.rb b/spec/finders/active_record_spec.rb index b39b011b2..18c4d3d9d 100644 --- a/spec/finders/active_record_spec.rb +++ b/spec/finders/active_record_spec.rb @@ -418,6 +418,13 @@ Developer.paginate((1..8).to_a, :per_page => 3, :page => 2, :order => 'id') }.should raise_error(ArgumentError) end + + it "errors out for invalid values" do |variable| + lambda { + # page that results in an offset larger than BIGINT + Project.page(307445734561825862) + }.should raise_error(WillPaginate::InvalidPage, "invalid offset: 9223372036854775830") + end protected