fix: make page[] param wrap consistent fixes #347

Author: Doug Smith

lib/json_api_client/paginating/paginator.rb

@@ -1,11 +1,41 @@
 module JsonApiClient
   module Paginating
     class Paginator
-      class_attribute :page_param,
-                      :per_page_param
+      # Define class accessors as methods to enforce standard way
+      # of defining pagination related query string params.
+      class << self
+        DEFAULT_PAGE_PARAM = "page".freeze
+        DEFAULT_PER_PAGE_PARAM = "per_page".freeze
+
+        def page_param
+          @_page_param ||= DEFAULT_PAGE_PARAM
+          "page[#{@_page_param}]"
+        end
+
+        def page_param=(param = DEFAULT_PAGE_PARAM)
+          raise ArgumentError, "don't wrap page_param" unless valid_param?(param)
+
+          @_page_param = param.to_s
+        end
+
+        def per_page_param
+          @_per_page_param ||= DEFAULT_PER_PAGE_PARAM
+          "page[#{@_per_page_param}]"
+        end
+
+        def per_page_param=(param = DEFAULT_PER_PAGE_PARAM)
+          raise ArgumentError, "don't wrap per_page_param" unless valid_param?(param)
+
+          @_per_page_param = param
+        end
 
-      self.page_param = "page"
-      self.per_page_param = "per_page"
+        private
+
+        def valid_param?(param)
+          !(param.nil? || param.to_s.include?("[") || param.to_s.include?("]"))
+        end
+
+      end
 
       attr_reader :params, :result_set, :links
 
@@ -75,6 +105,14 @@ def next_page
         current_page < total_pages ? (current_page + 1) : nil
       end
 
+      def page_param
+        self.class.page_param
+      end
+
+      def per_page_param
+        self.class.per_page_param
+      end
+
       alias limit_value per_page
 
       protected

lib/json_api_client/query/builder.rb

@@ -146,7 +146,7 @@ def primary_key_params
       end
 
       def pagination_params
-        @pagination_params.empty? ? {} : {page: @pagination_params}
+        @pagination_params
       end
 
       def includes_params

test/unit/top_level_links_test.rb

@@ -47,13 +47,13 @@ def test_can_parse_pagination_links
         }],
         links: {
           self: "http://example.com/articles",
-          next: "http://example.com/articles?page=2",
+          next: "http://example.com/articles?page[page]=2",
           prev: nil,
           first: "http://example.com/articles",
-          last: "http://example.com/articles?page=6"
+          last: "http://example.com/articles?page[page]=6"
         }
       }.to_json)
-    stub_request(:get, "http://example.com/articles?page=2")
+    stub_request(:get, "http://example.com/articles?page[page]=2")
       .to_return(headers: {content_type: "application/vnd.api+json"}, body: {
         data: [{
           type: "articles",
@@ -63,19 +63,19 @@ def test_can_parse_pagination_links
           }
         }],
         links: {
-          self: "http://example.com/articles?page=2",
-          next: "http://example.com/articles?page=3",
+          self: "http://example.com/articles?page[page]=2",
+          next: "http://example.com/articles?page[page]=3",
           prev: "http://example.com/articles",
           first: "http://example.com/articles",
-          last: "http://example.com/articles?page=6"
+          last: "http://example.com/articles?page[page]=6"
         }
       }.to_json)
 
     assert_pagination
   end
 
   def test_can_parse_pagination_links_with_custom_config
-    JsonApiClient::Paginating::Paginator.page_param = "page[number]"
+    JsonApiClient::Paginating::Paginator.page_param = "number"
 
     stub_request(:get, "http://example.com/articles")
       .to_return(headers: {content_type: "application/vnd.api+json"}, body: {
@@ -114,6 +114,7 @@ def test_can_parse_pagination_links_with_custom_config
 
     assert_pagination
 
+  ensure
     JsonApiClient::Paginating::Paginator.page_param = "page"
   end
 
@@ -131,7 +132,7 @@ def test_can_parse_pagination_links_when_no_next_page
           self: "http://example.com/articles",
           prev: nil,
           first: "http://example.com/articles",
-          last: "http://example.com/articles?page=1"
+          last: "http://example.com/articles?page[page]=1"
         }
       }.to_json)
 
@@ -154,7 +155,7 @@ def test_can_parse_complex_pagination_links
             meta: {}
           },
           next: {
-            href: "http://example.com/articles?page=2",
+            href: "http://example.com/articles?page[page]=2",
             meta: {}
           },
           prev: nil,
@@ -163,12 +164,12 @@ def test_can_parse_complex_pagination_links
             meta: {}
           },
           last: {
-            href: "http://example.com/articles?page=6",
+            href: "http://example.com/articles?page[page]=6",
             meta: {}
           }
         }
       }.to_json)
-    stub_request(:get, "http://example.com/articles?page=2")
+    stub_request(:get, "http://example.com/articles?page[page]=2")
       .to_return(headers: {content_type: "application/vnd.api+json"}, body: {
         data: [{
           type: "articles",
@@ -179,11 +180,11 @@ def test_can_parse_complex_pagination_links
         }],
         links: {
           self: {
-            href: "http://example.com/articles?page=2",
+            href: "http://example.com/articles?page[page]=2",
             meta: {}
           },
           next: {
-            href: "http://example.com/articles?page=3",
+            href: "http://example.com/articles?page[page]=3",
             meta: {}
           },
           prev: {
@@ -195,7 +196,7 @@ def test_can_parse_complex_pagination_links
             meta: {}
           },
           last: {
-            href: "http://example.com/articles?page=6",
+            href: "http://example.com/articles?page[page]=6",
             meta: {}
           }
         }