Make options endpoints and related components thread-safe

Author: ericproulx

lib/grape/dsl/desc.rb

@@ -49,7 +49,7 @@ module Desc
       #       # ...
       #     end
       #
-      def desc(description, options = {}, &config_block)
+      def desc(description, **options, &config_block)
         settings =
           if config_block
             endpoint_config = defined?(configuration) ? configuration : nil

lib/grape/dsl/inside_route.rb

@@ -152,6 +152,7 @@ def version
       end
 
       def configuration
+        pp options[:for].configuration
         options[:for].configuration.evaluate
       end
 

lib/grape/dsl/routing.rb

@@ -141,7 +141,7 @@ def mount(mounts, *opts)
       #       {hello: 'world'}
       #     end
       #   end
-      def route(methods, paths = ['/'], route_options = {}, &block)
+      def route(methods, paths = ['/'], **route_options, &block)
         method = methods == :any ? '*' : methods
         endpoint_params = inheritable_setting.namespace_stackable_with_hash(:params) || {}
         endpoint_description = inheritable_setting.route[:description]
@@ -156,7 +156,7 @@ def route(methods, paths = ['/'], route_options = {}, &block)
           route_options: all_route_options
         }
 
-        new_endpoint = Grape::Endpoint.new(inheritable_setting, endpoint_options, &block)
+        new_endpoint = Grape::Endpoint.new(inheritable_setting, **endpoint_options, &block)
         endpoints << new_endpoint unless endpoints.any? { |e| e.equals?(new_endpoint) }
 
         inheritable_setting.route_end
@@ -166,7 +166,7 @@ def route(methods, paths = ['/'], route_options = {}, &block)
       Grape::HTTP_SUPPORTED_METHODS.each do |supported_method|
         define_method supported_method.downcase do |*args, **options, &block|
           paths = args.first || ['/']
-          route(supported_method, paths, options, &block)
+          route(supported_method, paths, **options, &block)
         end
       end
 
@@ -182,7 +182,7 @@ def route(methods, paths = ['/'], route_options = {}, &block)
       #         # defines the endpoint: GET /foo/bar
       #       end
       #     end
-      def namespace(space = nil, options = {}, &block)
+      def namespace(space = nil, **options, &block)
         return Namespace.joined_space_path(inheritable_setting.namespace_stackable[:namespace]) unless space || block
 
         within_namespace do
@@ -217,9 +217,7 @@ def reset_endpoints!
       #
       # @param param [Symbol] The name of the parameter you wish to declare.
       # @option options [Regexp] You may supply a regular expression that the declared parameter must meet.
-      def route_param(param, options = {}, &block)
-        options = options.dup
-
+      def route_param(param, **options, &block)
         options[:requirements] = {
           param.to_sym => options[:requirements]
         } if options[:requirements].is_a?(Regexp)
@@ -228,7 +226,7 @@ def route_param(param, options = {}, &block)
           requires param, type: options[:type]
         end if options.key?(:type)
 
-        namespace(":#{param}", options, &block)
+        namespace(":#{param}", **options, &block)
       end
 
       # @return array of defined versions

lib/grape/endpoint.rb

@@ -46,10 +46,7 @@ def run_before_each(endpoint)
     # @note This happens at the time of API definition, so in this context the
     # endpoint does not know if it will be mounted under a different endpoint.
     # @yield a block defining what your API should do when this endpoint is hit
-    def initialize(new_settings, options = {}, &block)
-      require_option(options, :path)
-      require_option(options, :method)
-
+    def initialize(new_settings, **options, &block)
       self.inheritable_setting = new_settings.point_in_time_copy
 
       # now +namespace_stackable(:declared_params)+ contains all params defined for
@@ -61,12 +58,11 @@ def initialize(new_settings, options = {}, &block)
       inheritable_setting.namespace_stackable[:representations] = [] unless inheritable_setting.namespace_stackable[:representations]
       inheritable_setting.namespace_inheritable[:default_error_status] = 500 unless inheritable_setting.namespace_inheritable[:default_error_status]
 
-      @options = options
-
-      @options[:path] = Array(options[:path])
-      @options[:path] << '/' if options[:path].empty?
+      @options = options.dup
+      @options[:path] = Array(@options[:path])
+      @options[:path] << '/' if @options[:path].empty?
 
-      @options[:method] = Array(options[:method])
+      @options[:method] = Array(@options[:method])
       @options[:route_options] ||= {}
 
       @lazy_initialize_lock = Mutex.new
@@ -89,10 +85,6 @@ def inherit_settings(namespace_stackable)
       endpoints&.each { |e| e.inherit_settings(namespace_stackable) }
     end
 
-    def require_option(options, key)
-      raise Grape::Exceptions::MissingOption.new(key) unless options.key?(key)
-    end
-
     def routes
       @routes ||= endpoints&.collect(&:routes)&.flatten || to_routes
     end

spec/grape/dsl/desc_spec.rb

@@ -14,7 +14,7 @@
     it 'sets a description' do
       desc_text = 'The description'
       options = { message: 'none' }
-      subject.desc desc_text, options
+      subject.desc desc_text, **options
       expect(subject.namespace_setting(:description)).to eq(options.merge(description: desc_text))
       expect(subject.route_setting(:description)).to eq(options.merge(description: desc_text))
     end

spec/grape/dsl/routing_spec.rb

@@ -138,7 +138,7 @@ class << self
         expect(endpoint_options[:route_options]).to eq(foo: 'bar', fiz: 'baz', params: { nuz: 'naz' })
       end.and_yield
 
-      subject.route(:get, '/foo', { foo: 'bar' }, &proc {})
+      subject.route(:get, '/foo', foo: 'bar', &proc {})
     end
   end
 
@@ -257,20 +257,20 @@ class << self
     let(:regex) { /(.*)/ }
 
     it 'calls #namespace with given params' do
-      expect(subject).to receive(:namespace).with(':foo', {}).and_yield
-      subject.route_param('foo', {}, &proc {})
+      expect(subject).to receive(:namespace).with(':foo').and_yield
+      subject.route_param('foo', &proc {})
     end
 
     it 'nests requirements option under param name' do
       expect(subject).to receive(:namespace) do |_param, options|
         expect(options[:requirements][:foo]).to eq regex
       end
-      subject.route_param('foo', options, &proc {})
+      subject.route_param('foo', **options, &proc {})
     end
 
     it 'does not modify options parameter' do
       allow(subject).to receive(:namespace)
-      expect { subject.route_param('foo', options, &proc {}) }
+      expect { subject.route_param('foo', **options, &proc {}) }
         .not_to(change { options })
     end
   end

spec/grape/endpoint_spec.rb

@@ -64,10 +64,7 @@ def app
     it 'takes a settings stack, options, and a block' do
       p = proc {}
       expect do
-        described_class.new(Grape::Util::InheritableSetting.new, {
-                              path: '/',
-                              method: :get
-                            }, &p)
+        described_class.new(Grape::Util::InheritableSetting.new, path: '/', method: :get, &p)
       end.not_to raise_error
     end
   end
@@ -1136,7 +1133,7 @@ def memoized
   end
 
   describe '#inspect' do
-    subject { described_class.new(settings, options).inspect }
+    subject { described_class.new(settings, **options).inspect }
 
     let(:options) do
       {