proposal for shared params refs: ruby-grape#716

dspaeth-faber · Aug 25, 2014 · 1753d02 · 1753d02
1 parent 3411432
commit 1753d02
Show file tree

Hide file tree

Showing 7 changed files with 139 additions and 11 deletions.
diff --git a/README.md b/README.md
@@ -746,7 +746,7 @@ You can also define reusable `params` using shared helpers.
 
 ```ruby
 module SharedParams
-  extend Grape::API::Helpers
+  extend Grape::SharedParams
 
   params :period do
     optional :start_date
@@ -760,7 +760,7 @@ module SharedParams
 end
 
 class API < Grape::API
-  helpers SharedParams
+  include_params SharedParams
 
   desc "Get collection."
   params do

diff --git a/UPGRADING.md b/UPGRADING.md
@@ -3,6 +3,45 @@ Upgrading Grape
 
 ### Upgrading to >= 0.9.0 
 
+### Changes in Shared Params
+
+Extending a helper Module from `Grape::API::Helpers` to share parameters is deprecated. Instead you should use the following code:
+
+```ruby
+
+module SharedParams
+   extend Grape::SharedParams
+
+   params :period do
+       optional :start_date
+       optional :end_date
+   end
+
+   params :pagination do
+     optional :page, type: Integer
+     optional :per_page, type: Integer
+   end
+end
+
+class API < Grape::API
+  include_params SharedParams
+
+  desc "Get collection."
+  params do
+    use :period, :pagination
+  end
+
+  get do
+    Collection
+      .from(params[:start_date])
+      .to(params[:end_date])
+      .page(params[:page])
+      .per(params[:per_page])
+  end
+end
+
+```
+
 #### Changes in Authentication
 
 The following middleware classes have been removed:

diff --git a/lib/grape.rb b/lib/grape.rb
@@ -34,6 +34,8 @@ module Grape
   autoload :Validations,         'grape/validations'
   autoload :Request,             'grape/http/request'
 
+  autoload :SharedParams,        'grape/shared_params'
+
   module Exceptions
     autoload :Base,                           'grape/exceptions/base'
     autoload :Validation,                     'grape/exceptions/validation'

diff --git a/lib/grape/api/helpers.rb b/lib/grape/api/helpers.rb
@@ -1,7 +1,12 @@
 module Grape
   class API
-    module Helpers
-      include Grape::DSL::Helpers::BaseHelper
+    def self.const_missing(name)
+      if name.to_sym == 'Helpers'.to_sym
+        warn 'Grape::API::Helpers is deprecated use Grape::SharedParams and include_params instead'
+        const_set(name, ::Grape::DSL::Helpers::BaseHelper)
+      else
+        super
+      end
     end
   end
 end
diff --git a/lib/grape/dsl/validations.rb b/lib/grape/dsl/validations.rb
@@ -15,6 +15,25 @@ def params(&block)
           Grape::Validations::ParamsScope.new(api: self, type: Hash, &block)
         end
 
+        def shared_params(name, &block)
+          if block_given?
+            Grape::SharedParams.shared_params[name] ||= Module.new do
+              extend Grape::SharedParams
+            end
+            Grape::SharedParams.shared_params[name].class_eval(&block)
+          else
+            Grape::SharedParams.shared_params[name]
+          end
+        end
+
+        def include_params(*names_or_modules)
+          names_or_modules.each do |name_or_module|
+            mod = shared_params(name_or_module) unless name_or_module.respond_to? :api_changed
+            mod = name_or_module unless mod
+            mod.api_changed(self)
+          end
+        end
+
         def document_attribute(names, opts)
           @last_description ||= {}
           @last_description[:params] ||= {}

diff --git a/lib/grape/shared_params.rb b/lib/grape/shared_params.rb
@@ -0,0 +1,10 @@
+module Grape
+  module SharedParams
+    include Grape::DSL::Helpers::BaseHelper
+
+    module_function
+    def shared_params
+      @shared_params ||= {}
+    end
+  end
+end
diff --git a/spec/grape/api/helpers_spec.rb b/spec/grape/api/helpers_spec.rb
@@ -1,22 +1,35 @@
 require 'spec_helper'
 
 describe Grape::API::Helpers do
-  module SharedParams
-    extend Grape::API::Helpers
+  module Grape
+    class API
+      module HelpersSpec
+        module SharedParams
+          extend Grape::SharedParams
 
-    params :pagination do
-      optional :page, type: Integer
-      optional :size, type: Integer
+          params :pagination do
+            optional :page, type: Integer
+            optional :size, type: Integer
+          end
+        end
+      end
     end
   end
 
   subject do
     Class.new(Grape::API) do
-      helpers SharedParams
       format :json
 
+      shared_params 'a name' do
+        params :pagination_more do
+          optional :per_page, type: Integer
+        end
+      end
+
+      include_params Grape::API::HelpersSpec::SharedParams, 'a name'
       params do
         use :pagination
+        use :pagination_more
       end
       get do
         declared(params, include_missing: true)
@@ -31,6 +44,46 @@ def app
   it 'defines parameters' do
     get '/'
     expect(last_response.status).to eq 200
-    expect(last_response.body).to eq({ page: nil, size: nil }.to_json)
+    expect(last_response.body).to eq({ page: nil, size: nil, per_page: nil }.to_json)
+  end
+
+  context 'deprecated API' do
+    module Grape
+      class API
+        module HelpersSpec
+          module SharedParamsOld
+            extend Grape::API::Helpers
+
+            params :pagination do
+              optional :page, type: Integer
+              optional :size, type: Integer
+            end
+          end
+        end
+      end
+    end
+
+    subject do
+      Class.new(Grape::API) do
+        helpers Grape::API::HelpersSpec::SharedParamsOld
+        format :json
+
+        params do
+          use :pagination
+        end
+        get do
+          declared(params, include_missing: true)
+        end
+      end
+    end
+
+    def app
+      subject
+    end
+
+    it 'defines parameters' do
+      get '/'
+      expect(last_response.status).to eq 200
+    end
   end
 end