diff --git a/CHANGELOG.md b/CHANGELOG.md index 37ff1642b1..0a15293fb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * [#2500](https://github.com/ruby-grape/grape/pull/2500): Remove deprecated `file` method - [@ericproulx](https://github.com/ericproulx). * [#2501](https://github.com/ruby-grape/grape/pull/2501): Remove deprecated `except` and `proc` options in values validator - [@ericproulx](https://github.com/ericproulx). * [#2502](https://github.com/ruby-grape/grape/pull/2502): Remove deprecation `options` in `desc` - [@ericproulx](https://github.com/ericproulx). +* [#2512](https://github.com/ruby-grape/grape/pull/2512): Optimize hash alloc - [@ericproulx](https://github.com/ericproulx). * Your contribution here. #### Fixes diff --git a/lib/grape/api.rb b/lib/grape/api.rb index eaa351c9bf..2a3435aa38 100644 --- a/lib/grape/api.rb +++ b/lib/grape/api.rb @@ -91,7 +91,7 @@ def const_missing(*args) # For instance, a description could be done using: `desc configuration[:description]` if it may vary # depending on where the endpoint is mounted. Use with care, if you find yourself using configuration # too much, you may actually want to provide a new API rather than remount it. - def mount_instance(**opts) + def mount_instance(opts = {}) instance = Class.new(@base_parent) instance.configuration = Grape::Util::EndpointConfiguration.new(opts[:configuration] || {}) instance.base = self diff --git a/lib/grape/api/instance.rb b/lib/grape/api/instance.rb index ce290df7cc..c6a608713a 100644 --- a/lib/grape/api/instance.rb +++ b/lib/grape/api/instance.rb @@ -194,88 +194,52 @@ def cascade? # will return an HTTP 405 response for any HTTP method that the resource # cannot handle. def add_head_not_allowed_methods_and_options_methods - versioned_route_configs = collect_route_config_per_pattern # The paths we collected are prepared (cf. Path#prepare), so they # contain already versioning information when using path versioning. + all_routes = self.class.endpoints.map(&:routes).flatten + # Disable versioning so adding a route won't prepend versioning # informations again. - without_root_prefix do - without_versioning do - versioned_route_configs.each do |config| - next if config[:options][:matching_wildchar] - - allowed_methods = config[:methods].dup - - allowed_methods |= [Rack::HEAD] if !self.class.namespace_inheritable(:do_not_route_head) && allowed_methods.include?(Rack::GET) - - allow_header = (self.class.namespace_inheritable(:do_not_route_options) ? allowed_methods : [Rack::OPTIONS] | allowed_methods) - - config[:endpoint].options[:options_route_enabled] = true unless self.class.namespace_inheritable(:do_not_route_options) || allowed_methods.include?(Rack::OPTIONS) - - attributes = config.merge(allowed_methods: allowed_methods, allow_header: allow_header) - generate_not_allowed_method(config[:pattern], **attributes) - end - end - end + without_root_prefix_and_versioning { collect_route_config_per_pattern(all_routes) } end - def collect_route_config_per_pattern - all_routes = self.class.endpoints.map(&:routes).flatten + def collect_route_config_per_pattern(all_routes) routes_by_regexp = all_routes.group_by(&:pattern_regexp) # Build the configuration based on the first endpoint and the collection of methods supported. - routes_by_regexp.values.map do |routes| - last_route = routes.last # Most of the configuration is taken from the last endpoint - matching_wildchar = routes.any? { |route| route.request_method == '*' } - { - options: { matching_wildchar: matching_wildchar }, - pattern: last_route.pattern, - requirements: last_route.requirements, - path: last_route.origin, - endpoint: last_route.app, - methods: matching_wildchar ? Grape::Http::Headers::SUPPORTED_METHODS : routes.map(&:request_method) - } - end - end + routes_by_regexp.each_value do |routes| + last_route = routes.last # Most of the configuration is taken from the last endpoint + next if routes.any? { |route| route.request_method == '*' } - # Generate a route that returns an HTTP 405 response for a user defined - # path on methods not specified - def generate_not_allowed_method(pattern, allowed_methods: [], **attributes) - supported_methods = - if self.class.namespace_inheritable(:do_not_route_options) - Grape::Http::Headers::SUPPORTED_METHODS - else - Grape::Http::Headers::SUPPORTED_METHODS_WITHOUT_OPTIONS - end - not_allowed_methods = supported_methods - allowed_methods - @router.associate_routes(pattern, not_allowed_methods: not_allowed_methods, **attributes) + allowed_methods = routes.map(&:request_method) + allowed_methods |= [Rack::HEAD] if !self.class.namespace_inheritable(:do_not_route_head) && allowed_methods.include?(Rack::GET) + + allow_header = self.class.namespace_inheritable(:do_not_route_options) ? allowed_methods : [Rack::OPTIONS] | allowed_methods + last_route.app.options[:options_route_enabled] = true unless self.class.namespace_inheritable(:do_not_route_options) || allowed_methods.include?(Rack::OPTIONS) + + @router.associate_routes(last_route.pattern, { + endpoint: last_route.app, + allow_header: allow_header + }) + end end # Allows definition of endpoints that ignore the versioning configuration # used by the rest of your API. - def without_versioning(&_block) + def without_root_prefix_and_versioning old_version = self.class.namespace_inheritable(:version) old_version_options = self.class.namespace_inheritable(:version_options) + old_root_prefix = self.class.namespace_inheritable(:root_prefix) self.class.namespace_inheritable_to_nil(:version) self.class.namespace_inheritable_to_nil(:version_options) + self.class.namespace_inheritable_to_nil(:root_prefix) yield self.class.namespace_inheritable(:version, old_version) self.class.namespace_inheritable(:version_options, old_version_options) - end - - # Allows definition of endpoints that ignore the root prefix used by the - # rest of your API. - def without_root_prefix(&_block) - old_prefix = self.class.namespace_inheritable(:root_prefix) - - self.class.namespace_inheritable_to_nil(:root_prefix) - - yield - - self.class.namespace_inheritable(:root_prefix, old_prefix) + self.class.namespace_inheritable(:root_prefix, old_root_prefix) end end end diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index a7efcd4905..c88f7867a9 100644 --- a/lib/grape/dsl/inside_route.rb +++ b/lib/grape/dsl/inside_route.rb @@ -26,8 +26,8 @@ def self.post_filter_methods(type) # has completed module PostBeforeFilter def declared(passed_params, options = {}, declared_params = nil, params_nested_path = []) - options = options.reverse_merge(include_missing: true, include_parent_namespaces: true, evaluate_given: false) - declared_params ||= optioned_declared_params(**options) + options.reverse_merge!(include_missing: true, include_parent_namespaces: true, evaluate_given: false) + declared_params ||= optioned_declared_params(options[:include_parent_namespaces]) res = if passed_params.is_a?(Array) declared_array(passed_params, options, declared_params, params_nested_path) @@ -120,8 +120,8 @@ def optioned_param_key(declared_param, options) options[:stringify] ? declared_param.to_s : declared_param.to_sym end - def optioned_declared_params(**options) - declared_params = if options[:include_parent_namespaces] + def optioned_declared_params(include_parent_namespaces) + declared_params = if include_parent_namespaces # Declared params including parent namespaces route_setting(:declared_params) else @@ -199,10 +199,9 @@ def rack_response(message, status = 200, headers = { Rack::CONTENT_TYPE => conte # Redirect to a new url. # # @param url [String] The url to be redirect. - # @param options [Hash] The options used when redirect. - # :permanent, default false. - # :body, default a short message including the URL. - def redirect(url, permanent: false, body: nil, **_options) + # @param permanent [Boolean] default false. + # @param body default a short message including the URL. + def redirect(url, permanent: false, body: nil) body_message = body if permanent status 301 diff --git a/lib/grape/dsl/parameters.rb b/lib/grape/dsl/parameters.rb index 821da5d793..48f53bcb95 100644 --- a/lib/grape/dsl/parameters.rb +++ b/lib/grape/dsl/parameters.rb @@ -136,7 +136,7 @@ def requires(*attrs, &block) require_required_and_optional_fields(attrs.first, opts) else validate_attributes(attrs, opts, &block) - block ? new_scope(orig_attrs, &block) : push_declared_params(attrs, **opts.slice(:as)) + block ? new_scope(orig_attrs, &block) : push_declared_params(attrs, opts.slice(:as)) end end @@ -162,7 +162,7 @@ def optional(*attrs, &block) else validate_attributes(attrs, opts, &block) - block ? new_scope(orig_attrs, true, &block) : push_declared_params(attrs, **opts.slice(:as)) + block ? new_scope(orig_attrs, true, &block) : push_declared_params(attrs, opts.slice(:as)) end end diff --git a/lib/grape/dsl/routing.rb b/lib/grape/dsl/routing.rb index 812ddd1d08..8145726ada 100644 --- a/lib/grape/dsl/routing.rb +++ b/lib/grape/dsl/routing.rb @@ -175,19 +175,12 @@ def route(methods, paths = ['/'], route_options = {}, &block) # end # end def namespace(space = nil, options = {}, &block) - @namespace_description = nil unless instance_variable_defined?(:@namespace_description) && @namespace_description - - if space || block - within_namespace do - previous_namespace_description = @namespace_description - @namespace_description = (@namespace_description || {}).deep_merge(namespace_setting(:description) || {}) - nest(block) do - namespace_stackable(:namespace, Namespace.new(space, **options)) if space - end - @namespace_description = previous_namespace_description + return Namespace.joined_space_path(namespace_stackable(:namespace)) unless space || block + + within_namespace do + nest(block) do + namespace_stackable(:namespace, Namespace.new(space, options)) if space end - else - Namespace.joined_space_path(namespace_stackable(:namespace)) end end diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 5fdb03567a..6bd72c23c9 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -145,17 +145,16 @@ def reset_routes! end def mount_in(router) - if endpoints - endpoints.each { |e| e.mount_in(router) } - else - reset_routes! - routes.each do |route| - methods = [route.request_method] - methods << Rack::HEAD if !namespace_inheritable(:do_not_route_head) && route.request_method == Rack::GET - methods.each do |method| - route = Grape::Router::Route.new(method, route.origin, **route.attributes.to_h) unless route.request_method == method - router.append(route.apply(self)) - end + return endpoints.each { |e| e.mount_in(router) } if endpoints + + reset_routes! + routes.each do |route| + router.append(route.apply(self)) + next unless !namespace_inheritable(:do_not_route_head) && route.request_method == Rack::GET + + route.dup.then do |head_route| + head_route.convert_to_head_request! + router.append(head_route.apply(self)) end end end @@ -164,8 +163,9 @@ def to_routes route_options = prepare_default_route_attributes map_routes do |method, path| path = prepare_path(path) - params = merge_route_options(**route_options.merge(suffix: path.suffix)) - route = Router::Route.new(method, path.path, **params) + route_options[:suffix] = path.suffix + params = options[:route_options].merge(route_options) + route = Grape::Router::Route.new(method, path.path, params) route.apply(self) end.flatten end @@ -196,10 +196,6 @@ def prepare_version version.length == 1 ? version.first : version end - def merge_route_options(**default) - options[:route_options].clone.merge!(**default) - end - def map_routes options[:method].map { |method| options[:path].map { |path| yield method, path } } end @@ -259,9 +255,10 @@ def run run_filters befores, :before if (allowed_methods = env[Grape::Env::GRAPE_ALLOWED_METHODS]) - raise Grape::Exceptions::MethodNotAllowed.new(header.merge('Allow' => allowed_methods)) unless options? + allow_header_value = allowed_methods.join(', ') + raise Grape::Exceptions::MethodNotAllowed.new(header.merge('Allow' => allow_header_value)) unless options? - header Grape::Http::Headers::ALLOW, allowed_methods + header Grape::Http::Headers::ALLOW, allow_header_value response_object = '' status 204 else @@ -287,59 +284,6 @@ def run end end - def build_stack(helpers) - stack = Grape::Middleware::Stack.new - - content_types = namespace_stackable_with_hash(:content_types) - format = namespace_inheritable(:format) - - stack.use Rack::Head - stack.use Class.new(Grape::Middleware::Error), - helpers: helpers, - format: format, - content_types: content_types, - default_status: namespace_inheritable(:default_error_status), - rescue_all: namespace_inheritable(:rescue_all), - rescue_grape_exceptions: namespace_inheritable(:rescue_grape_exceptions), - default_error_formatter: namespace_inheritable(:default_error_formatter), - error_formatters: namespace_stackable_with_hash(:error_formatters), - rescue_options: namespace_stackable_with_hash(:rescue_options), - rescue_handlers: namespace_reverse_stackable_with_hash(:rescue_handlers), - base_only_rescue_handlers: namespace_stackable_with_hash(:base_only_rescue_handlers), - all_rescue_handler: namespace_inheritable(:all_rescue_handler), - grape_exceptions_rescue_handler: namespace_inheritable(:grape_exceptions_rescue_handler) - - stack.concat namespace_stackable(:middleware) - - if namespace_inheritable(:version).present? - stack.use Grape::Middleware::Versioner.using(namespace_inheritable(:version_options)[:using]), - versions: namespace_inheritable(:version).flatten, - version_options: namespace_inheritable(:version_options), - prefix: namespace_inheritable(:root_prefix), - mount_path: namespace_stackable(:mount_path).first - end - - stack.use Grape::Middleware::Formatter, - format: format, - default_format: namespace_inheritable(:default_format) || :txt, - content_types: content_types, - formatters: namespace_stackable_with_hash(:formatters), - parsers: namespace_stackable_with_hash(:parsers) - - builder = stack.build - builder.run ->(env) { env[Grape::Env::API_ENDPOINT].run } - builder.to_app - end - - def build_helpers - helpers = namespace_stackable(:helpers) - return if helpers.empty? - - Module.new { helpers.each { |mod_to_include| include mod_to_include } } - end - - private :build_stack, :build_helpers - def execute @block&.call(self) end @@ -411,7 +355,7 @@ def validations return enum_for(:validations) unless block_given? route_setting(:saved_validations)&.each do |saved_validation| - yield Grape::Validations::ValidatorFactory.create_validator(**saved_validation) + yield Grape::Validations::ValidatorFactory.create_validator(saved_validation) end end @@ -419,5 +363,58 @@ def options? options[:options_route_enabled] && env[Rack::REQUEST_METHOD] == Rack::OPTIONS end + + private + + def build_stack(helpers) + stack = Grape::Middleware::Stack.new + + content_types = namespace_stackable_with_hash(:content_types) + format = namespace_inheritable(:format) + + stack.use Rack::Head + stack.use Class.new(Grape::Middleware::Error), + helpers: helpers, + format: format, + content_types: content_types, + default_status: namespace_inheritable(:default_error_status), + rescue_all: namespace_inheritable(:rescue_all), + rescue_grape_exceptions: namespace_inheritable(:rescue_grape_exceptions), + default_error_formatter: namespace_inheritable(:default_error_formatter), + error_formatters: namespace_stackable_with_hash(:error_formatters), + rescue_options: namespace_stackable_with_hash(:rescue_options), + rescue_handlers: namespace_reverse_stackable_with_hash(:rescue_handlers), + base_only_rescue_handlers: namespace_stackable_with_hash(:base_only_rescue_handlers), + all_rescue_handler: namespace_inheritable(:all_rescue_handler), + grape_exceptions_rescue_handler: namespace_inheritable(:grape_exceptions_rescue_handler) + + stack.concat namespace_stackable(:middleware) + + if namespace_inheritable(:version).present? + stack.use Grape::Middleware::Versioner.using(namespace_inheritable(:version_options)[:using]), + versions: namespace_inheritable(:version).flatten, + version_options: namespace_inheritable(:version_options), + prefix: namespace_inheritable(:root_prefix), + mount_path: namespace_stackable(:mount_path).first + end + + stack.use Grape::Middleware::Formatter, + format: format, + default_format: namespace_inheritable(:default_format) || :txt, + content_types: content_types, + formatters: namespace_stackable_with_hash(:formatters), + parsers: namespace_stackable_with_hash(:parsers) + + builder = stack.build + builder.run ->(env) { env[Grape::Env::API_ENDPOINT].run } + builder.to_app + end + + def build_helpers + helpers = namespace_stackable(:helpers) + return if helpers.empty? + + Module.new { helpers.each { |mod_to_include| include mod_to_include } } + end end end diff --git a/lib/grape/exceptions/base.rb b/lib/grape/exceptions/base.rb index e262646c99..27ef78b456 100644 --- a/lib/grape/exceptions/base.rb +++ b/lib/grape/exceptions/base.rb @@ -9,7 +9,7 @@ class Base < StandardError attr_reader :status, :headers - def initialize(status: nil, message: nil, headers: nil, **_options) + def initialize(status: nil, message: nil, headers: nil) super(message) @status = status @@ -26,42 +26,32 @@ def [](index) # if BASE_ATTRIBUTES_KEY.key respond to a string message, then short_message is returned # if BASE_ATTRIBUTES_KEY.key respond to a Hash, means it may have problem , summary and resolution def compose_message(key, **attributes) - short_message = translate_message(key, **attributes) - if short_message.is_a? Hash - @problem = problem(key, **attributes) - @summary = summary(key, **attributes) - @resolution = resolution(key, **attributes) - [['Problem', @problem], ['Summary', @summary], ['Resolution', @resolution]].each_with_object(+'') do |detail_array, message| - message << "\n#{detail_array[0]}:\n #{detail_array[1]}" unless detail_array[1].blank? - message - end - else - short_message - end - end + short_message = translate_message(key, attributes) + return short_message unless short_message.is_a?(Hash) - def problem(key, **attributes) - translate_message(:"#{key}.problem", **attributes) + each_steps(key, attributes).with_object(+'') do |detail_array, message| + message << "\n#{detail_array[0]}:\n #{detail_array[1]}" unless detail_array[1].blank? + end end - def summary(key, **attributes) - translate_message(:"#{key}.summary", **attributes) - end + def each_steps(key, attributes) + return enum_for(:each_steps, key, attributes) unless block_given? - def resolution(key, **attributes) - translate_message(:"#{key}.resolution", **attributes) + yield 'Problem', translate_message(:"#{key}.problem", attributes) + yield 'Summary', translate_message(:"#{key}.summary", attributes) + yield 'Resolution', translate_message(:"#{key}.resolution", attributes) end - def translate_attributes(keys, **options) + def translate_attributes(keys, options = {}) keys.map do |key| - translate("#{BASE_ATTRIBUTES_KEY}.#{key}", default: key, **options) + translate("#{BASE_ATTRIBUTES_KEY}.#{key}", options.merge(default: key.to_s)) end.join(', ') end - def translate_message(key, **options) + def translate_message(key, options = {}) case key when Symbol - translate("#{BASE_MESSAGES_KEY}.#{key}", default: '', **options) + translate("#{BASE_MESSAGES_KEY}.#{key}", options.merge(default: '')) when Proc key.call else @@ -69,14 +59,12 @@ def translate_message(key, **options) end end - def translate(key, **options) - options = options.dup - options[:default] &&= options[:default].to_s + def translate(key, options) message = ::I18n.translate(key, **options) - message.presence || fallback_message(key, **options) + message.presence || fallback_message(key, options) end - def fallback_message(key, **options) + def fallback_message(key, options) if ::I18n.enforce_available_locales && ::I18n.available_locales.exclude?(FALLBACK_LOCALE) key else diff --git a/lib/grape/exceptions/validation.rb b/lib/grape/exceptions/validation.rb index 8d368d2776..0a9e9c5dd3 100644 --- a/lib/grape/exceptions/validation.rb +++ b/lib/grape/exceptions/validation.rb @@ -2,16 +2,17 @@ module Grape module Exceptions - class Validation < Grape::Exceptions::Base + class Validation < Base attr_accessor :params, :message_key - def initialize(params:, message: nil, **args) + def initialize(params:, message: nil, status: nil, headers: nil) @params = params if message @message_key = message if message.is_a?(Symbol) - args[:message] = translate_message(message) + message = translate_message(message) end - super(**args) + + super(status: status, message: message, headers: headers) end # Remove all the unnecessary stuff from Grape::Exceptions::Base like status diff --git a/lib/grape/exceptions/validation_errors.rb b/lib/grape/exceptions/validation_errors.rb index b8a843b1a5..8859d579b1 100644 --- a/lib/grape/exceptions/validation_errors.rb +++ b/lib/grape/exceptions/validation_errors.rb @@ -2,7 +2,7 @@ module Grape module Exceptions - class ValidationErrors < Grape::Exceptions::Base + class ValidationErrors < Base ERRORS_FORMAT_KEY = 'grape.errors.format' DEFAULT_ERRORS_FORMAT = '%s %s' @@ -10,7 +10,7 @@ class ValidationErrors < Grape::Exceptions::Base attr_reader :errors - def initialize(errors: [], headers: {}, **_options) + def initialize(errors: [], headers: {}) @errors = errors.group_by(&:params) super(message: full_messages.join(', '), status: 400, headers: headers) end diff --git a/lib/grape/middleware/versioner/header.rb b/lib/grape/middleware/versioner/header.rb index 619549304f..11cbfc7bcb 100644 --- a/lib/grape/middleware/versioner/header.rb +++ b/lib/grape/middleware/versioner/header.rb @@ -46,14 +46,10 @@ def match_best_quality_media_type! if media_type yield media_type else - fail!(allowed_methods) + fail! end end - def allowed_methods - env[Grape::Env::GRAPE_ALLOWED_METHODS] - end - def accept_header env[Grape::Http::Headers::HTTP_ACCEPT] end @@ -93,8 +89,8 @@ def invalid_version_header!(message) raise Grape::Exceptions::InvalidVersionHeader.new(message, error_headers) end - def fail!(grape_allowed_methods) - return grape_allowed_methods if grape_allowed_methods.present? + def fail! + return if env[Grape::Env::GRAPE_ALLOWED_METHODS].present? media_types = q_values_mime_types.map { |mime_type| Grape::Util::MediaType.parse(mime_type) } vendor_not_found!(media_types) || version_not_found!(media_types) diff --git a/lib/grape/namespace.rb b/lib/grape/namespace.rb index 537e7ff663..bdaa3a53f7 100644 --- a/lib/grape/namespace.rb +++ b/lib/grape/namespace.rb @@ -12,7 +12,7 @@ class Namespace # @option options :requirements [Hash] param-regex pairs, all of which must # be met by a request's params for all endpoints in this namespace, or # validation will fail and return a 422. - def initialize(space, **options) + def initialize(space, options) @space = space.to_s @options = options end diff --git a/lib/grape/request.rb b/lib/grape/request.rb index 7093ab95dd..b2a62053ea 100644 --- a/lib/grape/request.rb +++ b/lib/grape/request.rb @@ -6,8 +6,8 @@ class Request < Rack::Request alias rack_params params - def initialize(env, **options) - extend options[:build_params_with] || Grape.config.param_builder + def initialize(env, build_params_with: nil) + extend build_params_with || Grape.config.param_builder super(env) end diff --git a/lib/grape/router.rb b/lib/grape/router.rb index 61722011af..6889b42135 100644 --- a/lib/grape/router.rb +++ b/lib/grape/router.rb @@ -38,8 +38,8 @@ def append(route) map[route.request_method] << route end - def associate_routes(pattern, **options) - Grape::Router::GreedyRoute.new(pattern: pattern, **options).then do |greedy_route| + def associate_routes(pattern, options) + Grape::Router::GreedyRoute.new(pattern, options).then do |greedy_route| @neutral_regexes << greedy_route.to_regexp(@neutral_map.length) @neutral_map << greedy_route end @@ -107,7 +107,7 @@ def transaction(env) route = match?(input, '*') - return last_neighbor_route.endpoint.call(env) if last_neighbor_route && last_response_cascade && route + return last_neighbor_route.options[:endpoint].call(env) if last_neighbor_route && last_response_cascade && route last_response_cascade = cascade_or_return_response.call(process_route(route, env)) if route @@ -152,8 +152,8 @@ def greedy_match?(input) def call_with_allow_headers(env, route) prepare_env_from_route(env, route) - env[Grape::Env::GRAPE_ALLOWED_METHODS] = route.allow_header.join(', ').freeze - route.endpoint.call(env) + env[Grape::Env::GRAPE_ALLOWED_METHODS] = route.options[:allow_header] + route.options[:endpoint].call(env) end def prepare_env_from_route(env, route) diff --git a/lib/grape/router/base_route.rb b/lib/grape/router/base_route.rb index 9d19c87209..86439e908f 100644 --- a/lib/grape/router/base_route.rb +++ b/lib/grape/router/base_route.rb @@ -7,8 +7,8 @@ class BaseRoute attr_reader :index, :pattern, :options - def initialize(**options) - @options = ActiveSupport::OrderedOptions.new.update(options) + def initialize(options) + @options = options.is_a?(ActiveSupport::OrderedOptions) ? options : ActiveSupport::OrderedOptions.new.update(options) end alias attributes options diff --git a/lib/grape/router/greedy_route.rb b/lib/grape/router/greedy_route.rb index a999c1b906..c2fbcf8e87 100644 --- a/lib/grape/router/greedy_route.rb +++ b/lib/grape/router/greedy_route.rb @@ -6,9 +6,9 @@ module Grape class Router class GreedyRoute < BaseRoute - def initialize(pattern:, **options) + def initialize(pattern, options) @pattern = pattern - super(**options) + super(options) end # Grape::Router:Route defines params as a function diff --git a/lib/grape/router/pattern.rb b/lib/grape/router/pattern.rb index 7b5b5276f0..5761b1ea1e 100644 --- a/lib/grape/router/pattern.rb +++ b/lib/grape/router/pattern.rb @@ -13,9 +13,9 @@ class Pattern def_delegators :to_regexp, :=== alias match? === - def initialize(pattern, **options) + def initialize(pattern, options) @origin = pattern - @path = build_path(pattern, anchor: options[:anchor], suffix: options[:suffix]) + @path = build_path(pattern, options) @pattern = build_pattern(@path, options) @to_regexp = @pattern.to_regexp end @@ -33,15 +33,15 @@ def build_pattern(path, options) path, uri_decode: true, params: options[:params], - capture: extract_capture(**options) + capture: extract_capture(options) ) end - def build_path(pattern, anchor: false, suffix: nil) - PatternCache[[build_path_from_pattern(pattern, anchor: anchor), suffix]] + def build_path(pattern, options) + PatternCache[[build_path_from_pattern(pattern, options), options[:suffix]]] end - def extract_capture(**options) + def extract_capture(options) sliced_options = options .slice(:format, :version) .delete_if { |_k, v| v.blank? } @@ -51,10 +51,10 @@ def extract_capture(**options) options[:requirements].merge(sliced_options) end - def build_path_from_pattern(pattern, anchor: false) + def build_path_from_pattern(pattern, options) if pattern.end_with?('*path') pattern.dup.insert(pattern.rindex('/') + 1, '?') - elsif anchor + elsif options[:anchor] pattern elsif pattern.end_with?('/') "#{pattern}?*path" diff --git a/lib/grape/router/route.rb b/lib/grape/router/route.rb index 335ecb712f..244b0a4070 100644 --- a/lib/grape/router/route.rb +++ b/lib/grape/router/route.rb @@ -9,10 +9,14 @@ class Route < BaseRoute def_delegators :pattern, :path, :origin - def initialize(method, pattern, **options) + def initialize(method, pattern, options) @request_method = upcase_method(method) - @pattern = Grape::Router::Pattern.new(pattern, **options) - super(**options) + @pattern = Grape::Router::Pattern.new(pattern, options) + super(options) + end + + def convert_to_head_request! + @request_method = Rack::HEAD end def exec(env) diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb index b3c4268f61..36c6ed4d3c 100644 --- a/lib/grape/validations/params_scope.rb +++ b/lib/grape/validations/params_scope.rb @@ -174,16 +174,12 @@ def reset_index # Adds a parameter declaration to our list of validations. # @param attrs [Array] (see Grape::DSL::Parameters#requires) - def push_declared_params(attrs, **opts) - opts = opts.merge(declared_params_scope: self) unless opts.key?(:declared_params_scope) - if lateral? - @parent.push_declared_params(attrs, **opts) - else - push_renamed_param(full_path + [attrs.first], opts[:as]) \ - if opts && opts[:as] + def push_declared_params(attrs, opts = {}) + opts[:declared_params_scope] = self unless opts.key?(:declared_params_scope) + return @parent.push_declared_params(attrs, opts) if lateral? - @declared_params.concat(attrs.map { |attr| ::Grape::Validations::ParamsScope::Attr.new(attr, opts[:declared_params_scope]) }) - end + push_renamed_param(full_path + [attrs.first], opts[:as]) if opts[:as] + @declared_params.concat(attrs.map { |attr| ::Grape::Validations::ParamsScope::Attr.new(attr, opts[:declared_params_scope]) }) end # Get the full path of the parameter scope in the hierarchy. diff --git a/lib/grape/validations/validator_factory.rb b/lib/grape/validations/validator_factory.rb index 444fa0421e..0e2022d3af 100644 --- a/lib/grape/validations/validator_factory.rb +++ b/lib/grape/validations/validator_factory.rb @@ -3,12 +3,12 @@ module Grape module Validations class ValidatorFactory - def self.create_validator(**options) + def self.create_validator(options) options[:validator_class].new(options[:attributes], options[:options], options[:required], options[:params_scope], - **options[:opts]) + options[:opts]) end end end diff --git a/lib/grape/validations/validators/base.rb b/lib/grape/validations/validators/base.rb index 3dd49fd797..ee2dc4a87e 100644 --- a/lib/grape/validations/validators/base.rb +++ b/lib/grape/validations/validators/base.rb @@ -13,15 +13,14 @@ class Base # @param options [Object] implementation-dependent Validator options # @param required [Boolean] attribute(s) are required or optional # @param scope [ParamsScope] parent scope for this Validator - # @param opts [Array] additional validation options - def initialize(attrs, options, required, scope, *opts) + # @param opts [Hash] additional validation options + def initialize(attrs, options, required, scope, opts) @attrs = Array(attrs) @option = options @required = required @scope = scope - opts = opts.any? ? opts.shift : {} - @fail_fast = opts.fetch(:fail_fast, false) - @allow_blank = opts.fetch(:allow_blank, false) + @fail_fast = opts[:fail_fast] + @allow_blank = opts[:allow_blank] end # Validates a given request. diff --git a/lib/grape/validations/validators/coerce_validator.rb b/lib/grape/validations/validators/coerce_validator.rb index 979ad47c67..eaf7c40697 100644 --- a/lib/grape/validations/validators/coerce_validator.rb +++ b/lib/grape/validations/validators/coerce_validator.rb @@ -4,7 +4,7 @@ module Grape module Validations module Validators class CoerceValidator < Base - def initialize(attrs, options, required, scope, **opts) + def initialize(attrs, options, required, scope, opts) super @converter = if type.is_a?(Grape::Validations::Types::VariantCollectionCoercer) diff --git a/lib/grape/validations/validators/default_validator.rb b/lib/grape/validations/validators/default_validator.rb index 9a59e5da15..eba8c77302 100644 --- a/lib/grape/validations/validators/default_validator.rb +++ b/lib/grape/validations/validators/default_validator.rb @@ -4,7 +4,7 @@ module Grape module Validations module Validators class DefaultValidator < Base - def initialize(attrs, options, required, scope, **opts) + def initialize(attrs, options, required, scope, opts = {}) @default = options super end diff --git a/lib/grape/validations/validators/except_values_validator.rb b/lib/grape/validations/validators/except_values_validator.rb index b125c12cfb..298eb0ab91 100644 --- a/lib/grape/validations/validators/except_values_validator.rb +++ b/lib/grape/validations/validators/except_values_validator.rb @@ -4,7 +4,7 @@ module Grape module Validations module Validators class ExceptValuesValidator < Base - def initialize(attrs, options, required, scope, **opts) + def initialize(attrs, options, required, scope, opts) @except = options.is_a?(Hash) ? options[:value] : options super end diff --git a/lib/grape/validations/validators/length_validator.rb b/lib/grape/validations/validators/length_validator.rb index f844f047ec..c84b4c0962 100644 --- a/lib/grape/validations/validators/length_validator.rb +++ b/lib/grape/validations/validators/length_validator.rb @@ -4,7 +4,7 @@ module Grape module Validations module Validators class LengthValidator < Base - def initialize(attrs, options, required, scope, **opts) + def initialize(attrs, options, required, scope, opts) @min = options[:min] @max = options[:max] @is = options[:is] diff --git a/lib/grape/validations/validators/values_validator.rb b/lib/grape/validations/validators/values_validator.rb index cc2a641721..30cdeee7b2 100644 --- a/lib/grape/validations/validators/values_validator.rb +++ b/lib/grape/validations/validators/values_validator.rb @@ -4,7 +4,7 @@ module Grape module Validations module Validators class ValuesValidator < Base - def initialize(attrs, options, required, scope, **opts) + def initialize(attrs, options, required, scope, opts) @values = options.is_a?(Hash) ? options[:value] : options super end diff --git a/spec/grape/dsl/parameters_spec.rb b/spec/grape/dsl/parameters_spec.rb index 28ff1cce0b..8c7c1e96cb 100644 --- a/spec/grape/dsl/parameters_spec.rb +++ b/spec/grape/dsl/parameters_spec.rb @@ -20,7 +20,7 @@ def validate_attributes_reader @validate_attributes end - def push_declared_params(args, **_opts) + def push_declared_params(args, _opts) @push_declared_params = args end diff --git a/spec/grape/dsl/routing_spec.rb b/spec/grape/dsl/routing_spec.rb index 617980bb29..1b54ba913a 100644 --- a/spec/grape/dsl/routing_spec.rb +++ b/spec/grape/dsl/routing_spec.rb @@ -179,7 +179,7 @@ it 'creates a new namespace with given name and options' do expect(subject).to receive(:within_namespace).and_yield expect(subject).to receive(:nest).and_yield - expect(Grape::Namespace).to receive(:new).with(:foo, foo: 'bar').and_return(new_namespace) + expect(Grape::Namespace).to receive(:new).with(:foo, { foo: 'bar' }).and_return(new_namespace) expect(subject).to receive(:namespace_stackable).with(:namespace, new_namespace) subject.namespace :foo, foo: 'bar', &proc {} diff --git a/spec/grape/middleware/error_spec.rb b/spec/grape/middleware/error_spec.rb index 88eb88725e..cc8a735d4b 100644 --- a/spec/grape/middleware/error_spec.rb +++ b/spec/grape/middleware/error_spec.rb @@ -29,7 +29,7 @@ def call(_env) context = self Rack::Builder.app do use Spec::Support::EndpointFaker - use Grape::Middleware::Error, **opts # rubocop:disable RSpec/DescribedClass + use Grape::Middleware::Error, opts # rubocop:disable RSpec/DescribedClass run context.err_app end end diff --git a/spec/grape/middleware/versioner/accept_version_header_spec.rb b/spec/grape/middleware/versioner/accept_version_header_spec.rb index 4f44384980..6bcf7b14ad 100644 --- a/spec/grape/middleware/versioner/accept_version_header_spec.rb +++ b/spec/grape/middleware/versioner/accept_version_header_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true describe Grape::Middleware::Versioner::AcceptVersionHeader do - subject { described_class.new(app, **(@options || {})) } + subject { described_class.new(app, @options) } let(:app) { ->(env) { [200, env, env] } } diff --git a/spec/grape/middleware/versioner/header_spec.rb b/spec/grape/middleware/versioner/header_spec.rb index 1d686aae33..12fd837e00 100644 --- a/spec/grape/middleware/versioner/header_spec.rb +++ b/spec/grape/middleware/versioner/header_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true describe Grape::Middleware::Versioner::Header do - subject { described_class.new(app, **(@options || {})) } + subject { described_class.new(app, @options) } let(:app) { ->(env) { [200, env, env] } } diff --git a/spec/grape/middleware/versioner/param_spec.rb b/spec/grape/middleware/versioner/param_spec.rb index 00099dfc91..4e6bb4aa72 100644 --- a/spec/grape/middleware/versioner/param_spec.rb +++ b/spec/grape/middleware/versioner/param_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true describe Grape::Middleware::Versioner::Param do - subject { described_class.new(app, **options) } + subject { described_class.new(app, options) } let(:app) { ->(env) { [200, env, env[Grape::Env::API_VERSION]] } } let(:options) { {} } diff --git a/spec/grape/middleware/versioner/path_spec.rb b/spec/grape/middleware/versioner/path_spec.rb index 79aff53761..4593eca034 100644 --- a/spec/grape/middleware/versioner/path_spec.rb +++ b/spec/grape/middleware/versioner/path_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true describe Grape::Middleware::Versioner::Path do - subject { described_class.new(app, **options) } + subject { described_class.new(app, options) } let(:app) { ->(env) { [200, env, env[Grape::Env::API_VERSION]] } } let(:options) { {} } diff --git a/spec/grape/router/greedy_route_spec.rb b/spec/grape/router/greedy_route_spec.rb index 29e966280e..f93c013b4b 100644 --- a/spec/grape/router/greedy_route_spec.rb +++ b/spec/grape/router/greedy_route_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe Grape::Router::GreedyRoute do - let(:instance) { described_class.new(pattern: pattern, **options) } + let(:instance) { described_class.new(pattern, options) } let(:index) { 0 } let(:pattern) { :pattern } let(:params) do diff --git a/spec/integration/grape_entity/entity_spec.rb b/spec/integration/grape_entity/entity_spec.rb index cdac039a6e..eaea823823 100644 --- a/spec/integration/grape_entity/entity_spec.rb +++ b/spec/integration/grape_entity/entity_spec.rb @@ -348,7 +348,7 @@ def call(_env) opts = options Rack::Builder.app do use Spec::Support::EndpointFaker - use Grape::Middleware::Error, **opts + use Grape::Middleware::Error, opts run ErrApp end end diff --git a/spec/integration/hashie/hashie_spec.rb b/spec/integration/hashie/hashie_spec.rb index 73c97ce1e1..d90b771265 100644 --- a/spec/integration/hashie/hashie_spec.rb +++ b/spec/integration/hashie/hashie_spec.rb @@ -164,11 +164,9 @@ end describe 'when the build_params_with is set to Hashie' do - subject(:request_params) { Grape::Request.new(env, **opts).params } + subject(:request_params) { Grape::Request.new(env, build_params_with: Grape::Extensions::Hashie::Mash::ParamBuilder).params } context 'when the API includes a specific param builder' do - let(:opts) { { build_params_with: Grape::Extensions::Hashie::Mash::ParamBuilder } } - it { is_expected.to be_a(Hashie::Mash) } end end diff --git a/spec/shared/versioning_examples.rb b/spec/shared/versioning_examples.rb index 0215a7c440..9f42eeda18 100644 --- a/spec/shared/versioning_examples.rb +++ b/spec/shared/versioning_examples.rb @@ -7,7 +7,7 @@ subject.get :hello do "Version: #{request.env[Grape::Env::API_VERSION]}" end - versioned_get '/hello', 'v1', **macro_options + versioned_get '/hello', 'v1', macro_options expect(last_response.body).to eql 'Version: v1' end @@ -18,7 +18,7 @@ subject.get :hello do "Version: #{request.env[Grape::Env::API_VERSION]}" end - versioned_get '/hello', 'v1', **macro_options.merge(prefix: 'api') + versioned_get '/hello', 'v1', macro_options.merge(prefix: 'api') expect(last_response.body).to eql 'Version: v1' end @@ -34,14 +34,14 @@ end end - versioned_get '/awesome', 'v1', **macro_options + versioned_get '/awesome', 'v1', macro_options expect(last_response.status).to be 404 - versioned_get '/awesome', 'v2', **macro_options + versioned_get '/awesome', 'v2', macro_options expect(last_response.status).to be 200 - versioned_get '/legacy', 'v1', **macro_options + versioned_get '/legacy', 'v1', macro_options expect(last_response.status).to be 200 - versioned_get '/legacy', 'v2', **macro_options + versioned_get '/legacy', 'v2', macro_options expect(last_response.status).to be 404 end @@ -51,11 +51,11 @@ 'I exist' end - versioned_get '/awesome', 'v1', **macro_options + versioned_get '/awesome', 'v1', macro_options expect(last_response.status).to be 200 - versioned_get '/awesome', 'v2', **macro_options + versioned_get '/awesome', 'v2', macro_options expect(last_response.status).to be 200 - versioned_get '/awesome', 'v3', **macro_options + versioned_get '/awesome', 'v3', macro_options expect(last_response.status).to be 404 end @@ -74,10 +74,10 @@ end end - versioned_get '/version', 'v2', **macro_options + versioned_get '/version', 'v2', macro_options expect(last_response.status).to eq(200) expect(last_response.body).to eq('v2') - versioned_get '/version', 'v1', **macro_options + versioned_get '/version', 'v1', macro_options expect(last_response.status).to eq(200) expect(last_response.body).to eq('version v1') end @@ -98,11 +98,11 @@ end end - versioned_get '/version', 'v1', **macro_options.merge(prefix: subject.prefix) + versioned_get '/version', 'v1', macro_options.merge(prefix: subject.prefix) expect(last_response.status).to eq(200) expect(last_response.body).to eq('version v1') - versioned_get '/version', 'v2', **macro_options.merge(prefix: subject.prefix) + versioned_get '/version', 'v2', macro_options.merge(prefix: subject.prefix) expect(last_response.status).to eq(200) expect(last_response.body).to eq('v2') end @@ -133,11 +133,11 @@ end end - versioned_get '/version', 'v1', **macro_options.merge(prefix: subject.prefix) + versioned_get '/version', 'v1', macro_options.merge(prefix: subject.prefix) expect(last_response.status).to eq(200) expect(last_response.body).to eq('v1-version') - versioned_get '/version', 'v2', **macro_options.merge(prefix: subject.prefix) + versioned_get '/version', 'v2', macro_options.merge(prefix: subject.prefix) expect(last_response.status).to eq(200) expect(last_response.body).to eq('v2-version') end @@ -150,7 +150,7 @@ subject.get :api_version_with_version_param do params[:version] end - versioned_get '/api_version_with_version_param?version=1', 'v1', **macro_options + versioned_get '/api_version_with_version_param?version=1', 'v1', macro_options expect(last_response.body).to eql '1' end @@ -186,13 +186,13 @@ context 'v1' do it 'finds endpoint' do - versioned_get '/version', 'v1', **macro_options + versioned_get '/version', 'v1', macro_options expect(last_response.status).to eq(200) expect(last_response.body).to eq('v1') end it 'finds catch all' do - versioned_get '/whatever', 'v1', **macro_options + versioned_get '/whatever', 'v1', macro_options expect(last_response.status).to eq(200) expect(last_response.body).to end_with 'whatever' end @@ -200,13 +200,13 @@ context 'v2' do it 'finds endpoint' do - versioned_get '/version', 'v2', **macro_options + versioned_get '/version', 'v2', macro_options expect(last_response.status).to eq(200) expect(last_response.body).to eq('v2') end it 'finds catch all' do - versioned_get '/whatever', 'v2', **macro_options + versioned_get '/whatever', 'v2', macro_options expect(last_response.status).to eq(200) expect(last_response.body).to end_with 'whatever' end diff --git a/spec/support/versioned_helpers.rb b/spec/support/versioned_helpers.rb index 75e56e7d18..c4e841249e 100644 --- a/spec/support/versioned_helpers.rb +++ b/spec/support/versioned_helpers.rb @@ -6,7 +6,7 @@ module Support module Helpers # Returns the path with options[:version] prefixed if options[:using] is :path. # Returns normal path otherwise. - def versioned_path(**options) + def versioned_path(options) case options[:using] when :path File.join('/', options[:prefix] || '', options[:version], options[:path]) @@ -17,7 +17,7 @@ def versioned_path(**options) end end - def versioned_headers(**options) + def versioned_headers(options) case options[:using] when :path, :param {} @@ -37,9 +37,9 @@ def versioned_headers(**options) end end - def versioned_get(path, version_name, **version_options) - path = versioned_path(**version_options.merge(version: version_name, path: path)) - headers = versioned_headers(**version_options.merge(version: version_name)) + def versioned_get(path, version_name, version_options) + path = versioned_path(version_options.merge(version: version_name, path: path)) + headers = versioned_headers(version_options.merge(version: version_name)) params = {} params = { version_options[:parameter] => version_name } if version_options[:using] == :param get path, params, headers