From 89cf147e75c856178cc8e93bc00e5b7ff5264b1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohnic=CC=81?= Date: Sun, 7 Aug 2011 14:25:24 +0200 Subject: [PATCH] represent the concept of page number with the PageNumber class It handles validation of numbers and calculating the offset value. --- lib/will_paginate/active_record.rb | 10 +++-- lib/will_paginate/collection.rb | 5 ++- lib/will_paginate/data_mapper.rb | 8 ++-- lib/will_paginate/page_number.rb | 57 ++++++++++++++++++++++++++ lib/will_paginate/per_page.rb | 36 ----------------- spec/collection_spec.rb | 8 ---- spec/page_number_spec.rb | 65 ++++++++++++++++++++++++++++++ spec/per_page_spec.rb | 9 +++++ 8 files changed, 145 insertions(+), 53 deletions(-) create mode 100644 lib/will_paginate/page_number.rb create mode 100644 spec/page_number_spec.rb diff --git a/lib/will_paginate/active_record.rb b/lib/will_paginate/active_record.rb index 7d944adb1..ec2fd26a9 100644 --- a/lib/will_paginate/active_record.rb +++ b/lib/will_paginate/active_record.rb @@ -1,4 +1,5 @@ require 'will_paginate/per_page' +require 'will_paginate/page_number' require 'will_paginate/collection' require 'active_record' @@ -30,7 +31,7 @@ def per_page(value = nil) def limit(num) rel = super if rel.current_page - rel.offset ::WillPaginate.calculate_offset(rel.current_page, rel.limit_value) + rel.offset rel.current_page.to_offset(rel.limit_value).to_i else rel end @@ -117,7 +118,7 @@ def paginate(options) count_options = options.delete(:count) options.delete(:page) - rel = limit(per_page).page(pagenum) + rel = limit(per_page.to_i).page(pagenum) rel = rel.apply_finder_options(options) if options.any? rel.wp_count_options = count_options if count_options rel.total_entries = total.to_i unless total.blank? @@ -126,8 +127,9 @@ def paginate(options) def page(num) rel = scoped.extending(RelationMethods) - pagenum, per_page, offset = ::WillPaginate.process_values(num, rel.limit_value || self.per_page) - rel = rel.offset(offset) + pagenum = ::WillPaginate::PageNumber(num.nil? ? 1 : num) + per_page = rel.limit_value || self.per_page + rel = rel.offset(pagenum.to_offset(per_page).to_i) rel = rel.limit(per_page) unless rel.limit_value rel.current_page = pagenum rel diff --git a/lib/will_paginate/collection.rb b/lib/will_paginate/collection.rb index 4d7c9659b..f18940780 100644 --- a/lib/will_paginate/collection.rb +++ b/lib/will_paginate/collection.rb @@ -1,4 +1,5 @@ require 'will_paginate/per_page' +require 'will_paginate/page_number' module WillPaginate # = The key to pagination @@ -24,7 +25,7 @@ class Collection < Array # is best to do lazy counting; in other words, count *conditionally* after # populating the collection using the +replace+ method. def initialize(page, per_page = WillPaginate.per_page, total = nil) - @current_page = InvalidPage.validate(page, 'page') + @current_page = WillPaginate::PageNumber(page) @per_page = per_page.to_i self.total_entries = total if total end @@ -74,7 +75,7 @@ def out_of_bounds? # the offset is 30. This property is useful if you want to render ordinals # side by side with records in the view: simply start with offset + 1. def offset - (current_page - 1) * per_page + @current_page.to_offset(per_page).to_i end # current_page - 1 or nil if there is no previous page diff --git a/lib/will_paginate/data_mapper.rb b/lib/will_paginate/data_mapper.rb index e1586c4f5..3aff3fb97 100644 --- a/lib/will_paginate/data_mapper.rb +++ b/lib/will_paginate/data_mapper.rb @@ -1,14 +1,16 @@ require 'dm-core' require 'dm-aggregates' require 'will_paginate/per_page' +require 'will_paginate/page_number' require 'will_paginate/collection' module WillPaginate module DataMapper module Pagination def page(num) - pagenum, per_page, offset = ::WillPaginate.process_values(num, query.limit || self.per_page) - options = {:offset => offset} + pagenum = ::WillPaginate::PageNumber(num.nil? ? 1 : num) + per_page = query.limit || self.per_page + options = {:offset => pagenum.to_offset(per_page).to_i} options[:limit] = per_page unless query.limit col = new_collection(query.merge(options)) col.current_page = pagenum @@ -21,7 +23,7 @@ def paginate(options) per_page = options.delete(:per_page) || self.per_page options.delete(:page) - options[:limit] = per_page + options[:limit] = per_page.to_i all(options).page(pagenum) end diff --git a/lib/will_paginate/page_number.rb b/lib/will_paginate/page_number.rb new file mode 100644 index 000000000..7883b1230 --- /dev/null +++ b/lib/will_paginate/page_number.rb @@ -0,0 +1,57 @@ +require 'delegate' +require 'forwardable' + +module WillPaginate + # a module that page number exceptions are tagged with + module InvalidPage; end + + # integer representing a page number + class PageNumber < DelegateClass(Integer) + # a value larger than this is not supported in SQL queries + BIGINT = 9223372036854775807 + + extend Forwardable + + def initialize(value, name) + value = Integer(value) + if 'offset' == name ? (value < 0 or value > BIGINT) : value < 1 + raise RangeError, "invalid #{name}: #{value.inspect}" + end + @name = name + super(value) + rescue ArgumentError, TypeError, RangeError => error + error.extend InvalidPage + raise error + end + + alias_method :to_i, :__getobj__ + + def inspect + "#{@name} #{to_i}" + end + + def to_offset(per_page) + PageNumber.new((to_i - 1) * per_page.to_i, 'offset') + end + + def kind_of?(klass) + super || to_i.kind_of?(klass) + end + alias is_a? kind_of? + end + + # Ultrahax: makes `Fixnum === current_page` checks pass + Numeric.extend Module.new { + def ===(obj) + obj.instance_of? PageNumber or super + end + } + + # An idemptotent coercion method + def self.PageNumber(value, name = 'page') + case value + when PageNumber then value + else PageNumber.new(value, name) + end + end +end diff --git a/lib/will_paginate/per_page.rb b/lib/will_paginate/per_page.rb index 8060f2f47..29303e1dd 100644 --- a/lib/will_paginate/per_page.rb +++ b/lib/will_paginate/per_page.rb @@ -24,40 +24,4 @@ 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 6441e9b0a..98050dc7d 100644 --- a/spec/collection_spec.rb +++ b/spec/collection_spec.rb @@ -106,14 +106,6 @@ end end - it "should raise WillPaginate::InvalidPage on invalid input" do - [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 not respond to page_count anymore" do Proc.new { create.page_count }.should raise_error(NoMethodError) end diff --git a/spec/page_number_spec.rb b/spec/page_number_spec.rb new file mode 100644 index 000000000..1287116c6 --- /dev/null +++ b/spec/page_number_spec.rb @@ -0,0 +1,65 @@ +require 'spec_helper' +require 'will_paginate/page_number' + +describe WillPaginate::PageNumber do + describe "valid" do + subject { described_class.new('12', 'page') } + + it { should eq(12) } + its(:inspect) { should eq('page 12') } + it { should be_a(WillPaginate::PageNumber) } + it { should be_instance_of(WillPaginate::PageNumber) } + it { should be_a(Numeric) } + it { should be_a(Fixnum) } + it { should_not be_instance_of(Fixnum) } + + it "passes the PageNumber=== type check" do |variable| + (WillPaginate::PageNumber === subject).should be + end + + it "passes the Numeric=== type check" do |variable| + (Numeric === subject).should be + (Fixnum === subject).should be + end + end + + describe "invalid" do + def create(value, name = 'page') + described_class.new(value, name) + end + + it "errors out on non-int values" do + lambda { create(nil) }.should raise_error(WillPaginate::InvalidPage) + lambda { create('') }.should raise_error(WillPaginate::InvalidPage) + lambda { create('Schnitzel') }.should raise_error(WillPaginate::InvalidPage) + end + + it "errors out on zero or less" do + lambda { create(0) }.should raise_error(WillPaginate::InvalidPage) + lambda { create(-1) }.should raise_error(WillPaginate::InvalidPage) + end + + it "doesn't error out on zero for 'offset'" do + lambda { create(0, 'offset') }.should_not raise_error + lambda { create(-1, 'offset') }.should raise_error(WillPaginate::InvalidPage) + end + end + + describe "coercion method" do + it "defaults to 'page' name" do + num = WillPaginate::PageNumber(12) + num.inspect.should eq('page 12') + end + + it "accepts a custom name" do + num = WillPaginate::PageNumber(12, 'monkeys') + num.inspect.should eq('monkeys 12') + end + + it "doesn't affect PageNumber instances" do + num = WillPaginate::PageNumber(12) + num2 = WillPaginate::PageNumber(num) + num2.object_id.should eq(num.object_id) + end + end +end diff --git a/spec/per_page_spec.rb b/spec/per_page_spec.rb index 94482fff5..ece303f8a 100644 --- a/spec/per_page_spec.rb +++ b/spec/per_page_spec.rb @@ -18,6 +18,15 @@ class MyModel end end + it "casts values to int" do + WillPaginate.per_page = '10' + begin + MyModel.per_page.should == 10 + ensure + WillPaginate.per_page = 30 + end + end + it "has an explicit value" do MyModel.per_page = 12 begin