Merge branch 'dev'

ddnexus · Jul 2, 2019 · 9addfa3 · 9addfa3
2 parents 1a06640 + 296f97d
commit 9addfa3
Show file tree

Hide file tree

Showing 14 changed files with 148 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,15 @@
 # CHANGELOG
 
+## Version 3.3.1
+
+### Changes
+
+Namespacing of all exceptions, still backward compatible with legacy rescues
+
+### Commits
+
+- [530cb24](http://github.com/ddnexus/pagy/commit/530cb24): all the exceptions are namespaced and can return more information for easier rescue of special cases
+
 ## Version 3.3.0
 
 ### Changes

diff --git a/docs/api/pagy.md b/docs/api/pagy.md
@@ -81,15 +81,15 @@ They are all integers:
 
 ### Other Variables
 
-| Variable          | Description                                                                                                                                                                                      | Default            |
-|:------------------|:-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:-------------------|
-| `:size`           | the size of the page links to show: array of initial pages, before current page, after current page, final pages. _(see also [Control the page links](../how-to.md#controlling-the-page-links))_ | `[1,4,4,1]`        |
-| `:page_param`     | the name of the page param name used in the url. _(see [Customizing the page param](../how-to.md#customizing-the-page-param))_                                                                   | `:page`            |
-| `:params`         | the arbitrary param hash to add to the url. _(see [Customizing the params](../how-to.md#customizing-the-params))_                                                                                | `{}`               |
-| `:anchor`         | the arbitrary anchor string (including the "#") to add to the url. _(see [Customizing the params](../how-to.md#customizing-the-params))_                                                         | `""`               |
-| `:link_extra`     | the extra attributes string (formatted as a valid HTML attribute/value pairs) added to the page links _(see [Customizing the link attributes](../how-to.md#customizing-the-link-attributes))_    | `""`               |
-| `:i18n_key` | the i18n key to lookup the `item_name` that gets interpolated in a few helper outputs _(see [Customizing the item name](../how-to.md#customizing-the-item-name))_                    | `"pagy.item_name"` |
-| `:cycle`          | enable cycling/circular/infinite pagination: `true` sets `next` to `1` when the current page is the last page                                                                                    | `false`            |
+| Variable      | Description                                                                                                                                                                                      | Default            |
+|:--------------|:-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:-------------------|
+| `:size`       | the size of the page links to show: array of initial pages, before current page, after current page, final pages. _(see also [Control the page links](../how-to.md#controlling-the-page-links))_ | `[1,4,4,1]`        |
+| `:page_param` | the name of the page param name used in the url. _(see [Customizing the page param](../how-to.md#customizing-the-page-param))_                                                                   | `:page`            |
+| `:params`     | the arbitrary param hash to add to the url. _(see [Customizing the params](../how-to.md#customizing-the-params))_                                                                                | `{}`               |
+| `:anchor`     | the arbitrary anchor string (including the "#") to add to the url. _(see [Customizing the params](../how-to.md#customizing-the-params))_                                                         | `""`               |
+| `:link_extra` | the extra attributes string (formatted as a valid HTML attribute/value pairs) added to the page links _(see [Customizing the link attributes](../how-to.md#customizing-the-link-attributes))_    | `""`               |
+| `:i18n_key`   | the i18n key to lookup the `item_name` that gets interpolated in a few helper outputs _(see [Customizing the item name](../how-to.md#customizing-the-item-name))_                                | `"pagy.item_name"` |
+| `:cycle`      | enable cycling/circular/infinite pagination: `true` sets `next` to `1` when the current page is the last page                                                                                    | `false`            |
 
 There is no specific validation for non-instance variables.
 
@@ -150,3 +150,23 @@ Which means:
 - the `series` array contains always at least the page #`1`, which for a single page is also the current page, thus a string. With `size: []` series returns `[]`
 - `from` and `to` of an empty page are both `0`
 - `prev` and `next` of a single page (not necessary an empty one) are both `nil` (i.e. there is no other page)
+
+## Exceptions
+
+### Pagy::VariableError
+
+A subclass of `ArgumentError` that offers a few informations for easy exception handling when some of the variable passed to the constructor is not valid.
+
+```ruby
+rescue Pagy::VariableError => e
+  e.pagy     #=> the pagy object that raised the exception
+  e.variable #=> the offending variable symbol (e.g. :page)
+  e.value    #=> the value of the offending variable (e.g -3)
+end
+```
+
+Mostly useful if you want to rescue from bad user input (e.g. illegal URL manipulation).
+
+### Pagy::OverflowError
+
+A subclass of `Pagy::VariableError`: it is raised when the `:page` variable exceed the maximum possible value calculated for the `:count`, i.e. the `:page` variable is in the correct range, but it's not consistent with the current `:count`. That may happen when the `:count` has been reduced after a page link has been generated (e.g. some record has been just removed from the DB). See also the [overflow](../extras/overflow.md) extra.
diff --git a/docs/extras/navs.md b/docs/extras/navs.md
@@ -83,7 +83,7 @@ pagy, records = pagy(collection, steps: false )
 
 The above statement means that from `0` to `540` pixels width, Pagy will use the `[2,3,3,2]` size, from `540` to `720` it will use the `[3,5,5,3]` size and over `720` it will use the `[5,7,7,5]` size. (Read more about the `:size` variable in the [How to control the page links](../how-to.md#controlling-the-page-links) section).
 
-**IMPORTANT**: You can set any number of steps with any arbitrary width/size. The only requirement is that the `:steps` hash must contain always the `0` width or an `ArgumentError` exception will be raises.
+**IMPORTANT**: You can set any number of steps with any arbitrary width/size. The only requirement is that the `:steps` hash must contain always the `0` width or a `Pagy::VariableError` exception will be raised.
 
 #### Setting the right sizes
 

diff --git a/lib/config/pagy.rb b/lib/config/pagy.rb
@@ -1,7 +1,7 @@
 # encoding: utf-8
 # frozen_string_literal: true
 
-# Pagy initializer file (3.3.0)
+# Pagy initializer file (3.3.1)
 # Customize only what you really need and notice that Pagy works also without any of the following lines.
 # Should you just cherry pick part of this file, please maintain the require-order of the extras
 

diff --git a/lib/locales/utils/loader.rb b/lib/locales/utils/loader.rb
@@ -23,7 +23,7 @@
     arg[:filepath]  ||= Pagy.root.join('locales', "#{arg[:locale]}.yml")
     arg[:pluralize] ||= plurals[arg[:locale]]
     hash = YAML.load(File.read(arg[:filepath], encoding: 'UTF-8')) #rubocop:disable Security/YAMLLoad
-    hash.key?(arg[:locale]) or raise ArgumentError, %(Pagy::I18n.load: :locale "#{arg[:locale]}" not found in :filepath "#{arg[:filepath].inspect}")
+    hash.key?(arg[:locale]) or raise VariableError, %(expected :locale "#{arg[:locale]}" not found in :filepath "#{arg[:filepath].inspect}")
     i18n[arg[:locale]] = [flatten.call(hash[arg[:locale]]), arg[:pluralize]]
   end
 end
diff --git a/lib/pagy.rb b/lib/pagy.rb
@@ -4,9 +4,7 @@
 
 require 'pathname'
 
-class Pagy ; VERSION = '3.3.0'
-
-  class OverflowError < StandardError; attr_reader :pagy; def initialize(pagy) @pagy = pagy end; end
+class Pagy ; VERSION = '3.3.1'
 
   # Root pathname to get the path of Pagy files like templates or dictionaries
   def self.root; @root ||= Pathname.new(__FILE__).dirname.freeze end
@@ -21,7 +19,7 @@ def initialize(vars)
     @vars = VARS.merge(vars.delete_if{|_,v| v.nil? || v == '' })               # default vars + cleaned vars
     { count:0, items:1, outset:0, page:1 }.each do |k,min|                     # validate instance variables
       (@vars[k] && instance_variable_set(:"@#{k}", @vars[k].to_i) >= min) \
-         or raise(ArgumentError, "expected :#{k} >= #{min}; got #{@vars[k].inspect}")
+         or raise(VariableError.new(self), "expected :#{k} >= #{min}; got #{@vars[k].inspect}")
     end
     @pages = @last = [(@count.to_f / @items).ceil, 1].max                      # cardinal and ordinal meanings
     @page <= @last or raise(OverflowError.new(self), "expected :page in 1..#{@last}; got #{@page.inspect}")
@@ -36,7 +34,7 @@ def initialize(vars)
   # Return the array of page numbers and :gap items e.g. [1, :gap, 7, 8, "9", 10, 11, :gap, 36]
   def series(size=@vars[:size])
     (series = []) and size.empty? and return series
-    4.times{|i| (size[i]>=0 rescue nil) or raise(ArgumentError, "expected 4 items >= 0 in :size; got #{size.inspect}")}
+    4.times{|i| (size[i]>=0 rescue nil) or raise(VariableError.new(self), "expected 4 items >= 0 in :size; got #{size.inspect}")}
     [*0..size[0], *@page-size[1]..@page+size[2], *@last-size[3]+1..@last+1].sort!.each_cons(2) do |a, b|
       if    a<0 || a==b || a>@last                                        # skip out of range and duplicates
       elsif a+1 == b; series.push(a)                                      # no gap     -> no additions
@@ -51,3 +49,4 @@ def series(size=@vars[:size])
 
 require 'pagy/backend'
 require 'pagy/frontend'
+require 'pagy/exceptions'
diff --git a/lib/pagy/countless.rb b/lib/pagy/countless.rb
@@ -12,7 +12,7 @@ def initialize(vars={})
       @vars = VARS.merge(vars.delete_if{|_,v| v.nil? || v == '' })      # default vars + cleaned vars (can be overridden)
       { items:1, outset:0, page:1 }.each do |k,min|                     # validate instance variables
         (@vars[k] && instance_variable_set(:"@#{k}", @vars[k].to_i) >= min) \
-         or raise(ArgumentError, "expected :#{k} >= #{min}; got #{@vars[k].inspect}")
+         or raise(VariableError.new(self), "expected :#{k} >= #{min}; got #{@vars[k].inspect}")
       end
       @offset = @items * (@page - 1) + @outset                          # pagination offset + outset (initial offset)
     end

diff --git a/lib/pagy/exceptions.rb b/lib/pagy/exceptions.rb
@@ -0,0 +1,22 @@
+class Pagy
+
+  class VariableError < ArgumentError
+    attr_reader :pagy
+
+    def initialize(pagy)
+      @pagy = pagy
+    end
+
+    def variable
+      message =~ /expected :([\w]+)/
+      $1.to_sym if $1
+    end
+
+    def value
+      pagy.vars[variable]
+    end
+  end
+
+  class OverflowError < VariableError; end
+
+end
diff --git a/lib/pagy/extras/overflow.rb b/lib/pagy/extras/overflow.rb
@@ -28,7 +28,7 @@ def initialize_with_overflow(vars)
       @prev = @last                             # prev relative to the actual page
       extend(Series)                            # special series for :empty_page
     else
-      raise ArgumentError, "expected :overflow variable in [:last_page, :empty_page, :exception]; got #{@vars[:overflow].inspect}"
+      raise VariableError.new(self), "expected :overflow variable in [:last_page, :empty_page, :exception]; got #{@vars[:overflow].inspect}"
     end
   end
   alias_method :initialize, :initialize_with_overflow
@@ -62,7 +62,7 @@ def finalize_with_overflow(items)
           @vars[:size] = []                         # no page in the series
           self
         else
-          raise ArgumentError, "expected :overflow variable in [:empty_page, :exception]; got #{@vars[:overflow].inspect}"
+          raise VariableError.new(self), "expected :overflow variable in [:empty_page, :exception]; got #{@vars[:overflow].inspect}"
         end
       end
       alias_method :finalize, :finalize_with_overflow

diff --git a/lib/pagy/extras/shared.rb b/lib/pagy/extras/shared.rb
@@ -19,7 +19,7 @@ class Pagy
   # Notice: if :steps is false it will use the single {0 => @vars[:size]} size
   def sequels
     steps = @vars[:steps] || {0 => @vars[:size]}
-    steps.key?(0) or raise(ArgumentError, "expected :steps to define the 0 width; got #{steps.inspect}")
+    steps.key?(0) or raise(VariableError.new(self), "expected :steps to define the 0 width; got #{steps.inspect}")
     sequels = {}; steps.each {|width, size| sequels[width.to_s] = series(size)}; sequels
   end
 

diff --git a/test/pagy/exceptions_test.rb b/test/pagy/exceptions_test.rb
@@ -0,0 +1,67 @@
+# encoding: utf-8
+# frozen_string_literal: true
+
+require_relative '../test_helper'
+
+
+describe Pagy::VariableError do
+
+  describe "#variable and #value" do
+
+    it 'raises for non overflow pages' do
+      begin
+        Pagy.new(count: 1, page: 0)
+      rescue Pagy::VariableError => e
+        e.variable.must_equal :page
+        e.value.must_equal 0
+      end
+
+      begin
+        Pagy.new(count: 1, page: -10)
+      rescue Pagy::VariableError => e
+        e.variable.must_equal :page
+        e.value.must_equal(-10)
+      end
+
+      begin
+        Pagy.new(count: 1, page: 'string')
+      rescue Pagy::VariableError => e
+        e.variable.must_equal :page
+        e.value.must_equal 'string'
+      end
+    end
+
+    it 'raises for other variables' do
+      begin
+        Pagy.new(count: -10)
+      rescue Pagy::VariableError => e
+        e.variable.must_equal :count
+        e.value.must_equal(-10)
+      end
+      begin
+        Pagy.new(count: 'string')
+      rescue Pagy::VariableError => e
+        e.variable.must_equal :count
+        e.value.must_equal 'string'
+      end
+    end
+
+    it 'rescues as ArgumentError (backward compatible)' do
+      begin
+        Pagy.new(count: 1, page: -10)
+      rescue ArgumentError => e
+        e.variable.must_equal :page
+        e.value.must_equal(-10)
+      end
+
+      begin
+        Pagy.new(count: 'string')
+      rescue ArgumentError => e
+        e.variable.must_equal :count
+        e.value.must_equal 'string'
+      end
+    end
+
+  end
+
+end
diff --git a/test/pagy/extras/overflow_test.rb b/test/pagy/extras/overflow_test.rb
@@ -65,9 +65,9 @@
       pagy.prev.must_equal pagy.last
     end
 
-    it 'raises ArgumentError' do
-      proc { Pagy.new(vars.merge(overflow: :unknown)) }.must_raise ArgumentError
-      proc { Pagy::Countless.new(countless_vars.merge(overflow: :unknown)).finalize(0) }.must_raise ArgumentError
+    it 'raises Pagy::VariableError' do
+      proc { Pagy.new(vars.merge(overflow: :unknown)) }.must_raise Pagy::VariableError
+      proc { Pagy::Countless.new(countless_vars.merge(overflow: :unknown)).finalize(0) }.must_raise Pagy::VariableError
     end
 
   end

diff --git a/test/pagy/frontend_test.rb b/test/pagy/frontend_test.rb
@@ -78,7 +78,7 @@
 
     it 'loads custom :locale, :filepath and :pluralize' do
       proc{ Pagy::I18n.load(locale: 'xx') }.must_raise Errno::ENOENT
-      proc{ Pagy::I18n.load(locale: 'xx', filepath: Pagy.root.join('locales', 'en.yml'))}.must_raise ArgumentError
+      proc{ Pagy::I18n.load(locale: 'xx', filepath: Pagy.root.join('locales', 'en.yml'))}.must_raise Pagy::VariableError
       proc{ Pagy::I18n.load(locale: 'en', filepath: Pagy.root.join('locales', 'xx.yml')) }.must_raise Errno::ENOENT
       custom_dictionary = File.join(File.dirname(__FILE__), 'custom.yml')
       Pagy::I18n.load(locale: 'custom', filepath: custom_dictionary)

diff --git a/test/pagy_test.rb b/test/pagy_test.rb
@@ -24,11 +24,11 @@
       Pagy.new(count: 100, page: '2').must_be_instance_of Pagy
       Pagy.new(count: 100, page: '').must_be_instance_of Pagy
       Pagy.new(count: 100, items: '10').must_be_instance_of Pagy
-      proc { Pagy.new({}) }.must_raise ArgumentError
-      proc { Pagy.new(count: 100, page: 0) }.must_raise ArgumentError
-      proc { Pagy.new(count: 100, page: 2, items: 0) }.must_raise ArgumentError
-      proc { Pagy.new(count: 100, page: 2, size: [1, 2, 3]).series }.must_raise ArgumentError
-      proc { Pagy.new(count: 100, page: 2, size: [1, 2, 3, '4']).series }.must_raise ArgumentError
+      proc { Pagy.new({}) }.must_raise Pagy::VariableError
+      proc { Pagy.new(count: 100, page: 0) }.must_raise Pagy::VariableError
+      proc { Pagy.new(count: 100, page: 2, items: 0) }.must_raise Pagy::VariableError
+      proc { Pagy.new(count: 100, page: 2, size: [1, 2, 3]).series }.must_raise Pagy::VariableError
+      proc { Pagy.new(count: 100, page: 2, size: [1, 2, 3, '4']).series }.must_raise Pagy::VariableError
       proc { Pagy.new(count: 100, page: '11') }.must_raise Pagy::OverflowError
       e = proc { Pagy.new(count: 100, page: 12) }.must_raise Pagy::OverflowError
       e.pagy.must_be_instance_of Pagy