diff --git a/CHANGELOG.md b/CHANGELOG.md index d274b058ea..6ec8b6d643 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ * [#2512](https://github.com/ruby-grape/grape/pull/2512): Optimize hash alloc - [@ericproulx](https://github.com/ericproulx). * [#2513](https://github.com/ruby-grape/grape/pull/2513): Optimize Grape::Path - [@ericproulx](https://github.com/ericproulx). * [#2514](https://github.com/ruby-grape/grape/pull/2514): Add rails 8.0 to CI - [@ericproulx](https://github.com/ericproulx). +* [#2516](https://github.com/ruby-grape/grape/pull/2516): Dynamic registration for parsers, formatters, versioners - [@ericproulx](https://github.com/ericproulx). * Your contribution here. #### Fixes diff --git a/lib/grape/api.rb b/lib/grape/api.rb index 2a3435aa38..8ce789c25b 100644 --- a/lib/grape/api.rb +++ b/lib/grape/api.rb @@ -78,15 +78,6 @@ def call(...) instance_for_rack.call(...) end - # Alleviates problems with autoloading by tring to search for the constant - def const_missing(*args) - if base_instance.const_defined?(*args) - base_instance.const_get(*args) - else - super - end - end - # The remountable class can have a configuration hash to provide some dynamic class-level variables. # 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 diff --git a/lib/grape/error_formatter.rb b/lib/grape/error_formatter.rb index 7784055b6a..7d9ace8a88 100644 --- a/lib/grape/error_formatter.rb +++ b/lib/grape/error_formatter.rb @@ -2,22 +2,14 @@ module Grape module ErrorFormatter - module_function + extend Grape::Util::Registry - DEFAULTS = { - serializable_hash: Grape::ErrorFormatter::Json, - json: Grape::ErrorFormatter::Json, - jsonapi: Grape::ErrorFormatter::Json, - txt: Grape::ErrorFormatter::Txt, - xml: Grape::ErrorFormatter::Xml - }.freeze + module_function def formatter_for(format, error_formatters = nil, default_error_formatter = nil) - select_formatter(error_formatters, format) || default_error_formatter || DEFAULTS[:txt] - end + return error_formatters[format] if error_formatters&.key?(format) - def select_formatter(error_formatters, format) - error_formatters&.key?(format) ? error_formatters[format] : DEFAULTS[format] + registry[format] || default_error_formatter || Grape::ErrorFormatter::Txt end end end diff --git a/lib/grape/error_formatter/base.rb b/lib/grape/error_formatter/base.rb index 2a1e758eba..f2cf223c67 100644 --- a/lib/grape/error_formatter/base.rb +++ b/lib/grape/error_formatter/base.rb @@ -2,36 +2,66 @@ module Grape module ErrorFormatter - module Base - def present(message, env) - present_options = {} - presented_message = message - if presented_message.is_a?(Hash) - presented_message = presented_message.dup - present_options[:with] = presented_message.delete(:with) + class Base + class << self + def call(message, backtrace, options = {}, env = nil, original_exception = nil) + merge_backtrace = backtrace.present? && options.dig(:rescue_options, :backtrace) + merge_original_exception = original_exception && options.dig(:rescue_options, :original_exception) + + wrapped_message = wrap_message(present(message, env)) + if wrapped_message.is_a?(Hash) + wrapped_message[:backtrace] = backtrace if merge_backtrace + wrapped_message[:original_exception] = original_exception.inspect if merge_original_exception + end + + format_structured_message(wrapped_message) end - presenter = env[Grape::Env::API_ENDPOINT].entity_class_for_obj(presented_message, present_options) + def present(message, env) + present_options = {} + presented_message = message + if presented_message.is_a?(Hash) + presented_message = presented_message.dup + present_options[:with] = presented_message.delete(:with) + end + + presenter = env[Grape::Env::API_ENDPOINT].entity_class_for_obj(presented_message, present_options) + + unless presenter || env[Grape::Env::GRAPE_ROUTING_ARGS].nil? + # env['api.endpoint'].route does not work when the error occurs within a middleware + # the Endpoint does not have a valid env at this moment + http_codes = env[Grape::Env::GRAPE_ROUTING_ARGS][:route_info].http_codes || [] + + found_code = http_codes.find do |http_code| + (http_code[0].to_i == env[Grape::Env::API_ENDPOINT].status) && http_code[2].respond_to?(:represent) + end if env[Grape::Env::API_ENDPOINT].request - unless presenter || env[Grape::Env::GRAPE_ROUTING_ARGS].nil? - # env['api.endpoint'].route does not work when the error occurs within a middleware - # the Endpoint does not have a valid env at this moment - http_codes = env[Grape::Env::GRAPE_ROUTING_ARGS][:route_info].http_codes || [] + presenter = found_code[2] if found_code + end - found_code = http_codes.find do |http_code| - (http_code[0].to_i == env[Grape::Env::API_ENDPOINT].status) && http_code[2].respond_to?(:represent) - end if env[Grape::Env::API_ENDPOINT].request + if presenter + embeds = { env: env } + embeds[:version] = env[Grape::Env::API_VERSION] if env.key?(Grape::Env::API_VERSION) + presented_message = presenter.represent(presented_message, embeds).serializable_hash + end - presenter = found_code[2] if found_code + presented_message end - if presenter - embeds = { env: env } - embeds[:version] = env[Grape::Env::API_VERSION] if env.key?(Grape::Env::API_VERSION) - presented_message = presenter.represent(presented_message, embeds).serializable_hash + def wrap_message(message) + return message if message.is_a?(Hash) + + { message: message } + end + + def format_structured_message(_structured_message) + raise NotImplementedError end - presented_message + def inherited(klass) + super + ErrorFormatter.register(klass) + end end end end diff --git a/lib/grape/error_formatter/json.rb b/lib/grape/error_formatter/json.rb index f4df46e849..bed5bd39df 100644 --- a/lib/grape/error_formatter/json.rb +++ b/lib/grape/error_formatter/json.rb @@ -2,28 +2,19 @@ module Grape module ErrorFormatter - module Json - extend Base - + class Json < Base class << self - def call(message, backtrace, options = {}, env = nil, original_exception = nil) - result = wrap_message(present(message, env)) - - result = merge_rescue_options(result, backtrace, options, original_exception) if result.is_a?(Hash) - - ::Grape::Json.dump(result) + def format_structured_message(structured_message) + ::Grape::Json.dump(structured_message) end private def wrap_message(message) - if message.is_a?(Hash) - message - elsif message.is_a?(Exceptions::ValidationErrors) - message.as_json - else - { error: ensure_utf8(message) } - end + return message if message.is_a?(Hash) + return message.as_json if message.is_a?(Exceptions::ValidationErrors) + + { error: ensure_utf8(message) } end def ensure_utf8(message) @@ -31,14 +22,6 @@ def ensure_utf8(message) message.encode('UTF-8', invalid: :replace, undef: :replace) end - - def merge_rescue_options(result, backtrace, options, original_exception) - rescue_options = options[:rescue_options] || {} - result = result.merge(backtrace: backtrace) if rescue_options[:backtrace] && backtrace && !backtrace.empty? - result = result.merge(original_exception: original_exception.inspect) if rescue_options[:original_exception] && original_exception - - result - end end end end diff --git a/lib/grape/error_formatter/jsonapi.rb b/lib/grape/error_formatter/jsonapi.rb new file mode 100644 index 0000000000..ed1d2d30f7 --- /dev/null +++ b/lib/grape/error_formatter/jsonapi.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module Grape + module ErrorFormatter + class Jsonapi < Json; end + end +end diff --git a/lib/grape/error_formatter/serializable_hash.rb b/lib/grape/error_formatter/serializable_hash.rb new file mode 100644 index 0000000000..14b9ed5972 --- /dev/null +++ b/lib/grape/error_formatter/serializable_hash.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module Grape + module ErrorFormatter + class SerializableHash < Json; end + end +end diff --git a/lib/grape/error_formatter/txt.rb b/lib/grape/error_formatter/txt.rb index 22fc5c5385..7c0c759184 100644 --- a/lib/grape/error_formatter/txt.rb +++ b/lib/grape/error_formatter/txt.rb @@ -2,26 +2,19 @@ module Grape module ErrorFormatter - module Txt - extend Base - - class << self - def call(message, backtrace, options = {}, env = nil, original_exception = nil) - message = present(message, env) - - result = message.is_a?(Hash) ? ::Grape::Json.dump(message) : message - Array.wrap(result).tap do |final_result| - rescue_options = options[:rescue_options] || {} - if rescue_options[:backtrace] && backtrace.present? - final_result << 'backtrace:' - final_result.concat(backtrace) - end - if rescue_options[:original_exception] && original_exception - final_result << 'original exception:' - final_result << original_exception.inspect - end - end.join("\r\n ") - end + class Txt < Base + def self.format_structured_message(structured_message) + message = structured_message[:message] || Grape::Json.dump(structured_message) + Array.wrap(message).tap do |final_message| + if structured_message.key?(:backtrace) + final_message << 'backtrace:' + final_message.concat(structured_message[:backtrace]) + end + if structured_message.key?(:original_exception) + final_message << 'original exception:' + final_message << structured_message[:original_exception] + end + end.join("\r\n ") end end end diff --git a/lib/grape/error_formatter/xml.rb b/lib/grape/error_formatter/xml.rb index e423c2fd9d..c78f6d650c 100644 --- a/lib/grape/error_formatter/xml.rb +++ b/lib/grape/error_formatter/xml.rb @@ -2,19 +2,9 @@ module Grape module ErrorFormatter - module Xml - extend Base - - class << self - def call(message, backtrace, options = {}, env = nil, original_exception = nil) - message = present(message, env) - - result = message.is_a?(Hash) ? message : { message: message } - rescue_options = options[:rescue_options] || {} - result = result.merge(backtrace: backtrace) if rescue_options[:backtrace] && backtrace && !backtrace.empty? - result = result.merge(original_exception: original_exception.inspect) if rescue_options[:original_exception] && original_exception - result.respond_to?(:to_xml) ? result.to_xml(root: :error) : result.to_s - end + class Xml < Base + def self.format_structured_message(structured_message) + structured_message.respond_to?(:to_xml) ? structured_message.to_xml(root: :error) : structured_message.to_s end end end diff --git a/lib/grape/formatter.rb b/lib/grape/formatter.rb index d586b0bd6a..6d5affb34f 100644 --- a/lib/grape/formatter.rb +++ b/lib/grape/formatter.rb @@ -2,24 +2,16 @@ module Grape module Formatter - module_function + extend Grape::Util::Registry - DEFAULTS = { - json: Grape::Formatter::Json, - jsonapi: Grape::Formatter::Json, - serializable_hash: Grape::Formatter::SerializableHash, - txt: Grape::Formatter::Txt, - xml: Grape::Formatter::Xml - }.freeze + module_function DEFAULT_LAMBDA_FORMATTER = ->(obj, _env) { obj } def formatter_for(api_format, formatters) - select_formatter(formatters, api_format) || DEFAULT_LAMBDA_FORMATTER - end + return formatters[api_format] if formatters&.key?(api_format) - def select_formatter(formatters, api_format) - formatters&.key?(api_format) ? formatters[api_format] : DEFAULTS[api_format] + registry[api_format] || DEFAULT_LAMBDA_FORMATTER end end end diff --git a/lib/grape/formatter/base.rb b/lib/grape/formatter/base.rb new file mode 100644 index 0000000000..dfd56d65d4 --- /dev/null +++ b/lib/grape/formatter/base.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Grape + module Formatter + class Base + def self.call(_object, _env) + raise NotImplementedError + end + + def self.inherited(klass) + super + Formatter.register(klass) + end + end + end +end diff --git a/lib/grape/formatter/json.rb b/lib/grape/formatter/json.rb index 0f0152f6c4..bfdd2ca286 100644 --- a/lib/grape/formatter/json.rb +++ b/lib/grape/formatter/json.rb @@ -2,13 +2,11 @@ module Grape module Formatter - module Json - class << self - def call(object, _env) - return object.to_json if object.respond_to?(:to_json) + class Json < Base + def self.call(object, _env) + return object.to_json if object.respond_to?(:to_json) - ::Grape::Json.dump(object) - end + ::Grape::Json.dump(object) end end end diff --git a/lib/grape/formatter/serializable_hash.rb b/lib/grape/formatter/serializable_hash.rb index cb9bfb7c14..5b29ece151 100644 --- a/lib/grape/formatter/serializable_hash.rb +++ b/lib/grape/formatter/serializable_hash.rb @@ -2,7 +2,7 @@ module Grape module Formatter - module SerializableHash + class SerializableHash < Base class << self def call(object, _env) return object if object.is_a?(String) diff --git a/lib/grape/formatter/txt.rb b/lib/grape/formatter/txt.rb index 1f1f9ef2f5..cb77e4f07f 100644 --- a/lib/grape/formatter/txt.rb +++ b/lib/grape/formatter/txt.rb @@ -2,11 +2,9 @@ module Grape module Formatter - module Txt - class << self - def call(object, _env) - object.respond_to?(:to_txt) ? object.to_txt : object.to_s - end + class Txt < Base + def self.call(object, _env) + object.respond_to?(:to_txt) ? object.to_txt : object.to_s end end end diff --git a/lib/grape/formatter/xml.rb b/lib/grape/formatter/xml.rb index c170d1843e..de0531758a 100644 --- a/lib/grape/formatter/xml.rb +++ b/lib/grape/formatter/xml.rb @@ -2,13 +2,11 @@ module Grape module Formatter - module Xml - class << self - def call(object, _env) - return object.to_xml if object.respond_to?(:to_xml) + class Xml < Base + def self.call(object, _env) + return object.to_xml if object.respond_to?(:to_xml) - raise Grape::Exceptions::InvalidFormatter.new(object.class, 'xml') - end + raise Grape::Exceptions::InvalidFormatter.new(object.class, 'xml') end end end diff --git a/lib/grape/middleware/versioner.rb b/lib/grape/middleware/versioner.rb index 1589f9b76c..bcca9fe596 100644 --- a/lib/grape/middleware/versioner.rb +++ b/lib/grape/middleware/versioner.rb @@ -11,14 +11,16 @@ module Grape module Middleware module Versioner + extend Grape::Util::Registry + module_function # @param strategy [Symbol] :path, :header, :accept_version_header or :param # @return a middleware class based on strategy def using(strategy) - Grape::Middleware::Versioner.const_get(:"#{strategy.to_s.camelize}") - rescue NameError - raise Grape::Exceptions::InvalidVersionerOption, strategy + raise Grape::Exceptions::InvalidVersionerOption, strategy unless registry.key?(strategy) + + registry[strategy] end end end diff --git a/lib/grape/middleware/versioner/accept_version_header.rb b/lib/grape/middleware/versioner/accept_version_header.rb index 1cf2bb6748..ff76a73723 100644 --- a/lib/grape/middleware/versioner/accept_version_header.rb +++ b/lib/grape/middleware/versioner/accept_version_header.rb @@ -17,8 +17,6 @@ module Versioner # X-Cascade header to alert Grape::Router to attempt the next matched # route. class AcceptVersionHeader < Base - include VersionerHelpers - def before potential_version = env[Grape::Http::Headers::HTTP_ACCEPT_VERSION]&.strip not_acceptable!('Accept-Version header must be set.') if strict? && potential_version.blank? diff --git a/lib/grape/middleware/versioner/base.rb b/lib/grape/middleware/versioner/base.rb new file mode 100644 index 0000000000..68604f14e6 --- /dev/null +++ b/lib/grape/middleware/versioner/base.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +module Grape + module Middleware + module Versioner + class Base < Grape::Middleware::Base + DEFAULT_PATTERN = /.*/i.freeze + DEFAULT_PARAMETER = 'apiver' + + def self.inherited(klass) + super + Versioner.register(klass) + end + + def default_options + { + versions: nil, + prefix: nil, + mount_path: nil, + pattern: DEFAULT_PATTERN, + version_options: { + strict: false, + cascade: true, + parameter: DEFAULT_PARAMETER + } + } + end + + def versions + options[:versions] + end + + def prefix + options[:prefix] + end + + def mount_path + options[:mount_path] + end + + def pattern + options[:pattern] + end + + def version_options + options[:version_options] + end + + def strict? + version_options[:strict] + end + + # By default those errors contain an `X-Cascade` header set to `pass`, which allows nesting and stacking + # of routes (see Grape::Router) for more information). To prevent + # this behavior, and not add the `X-Cascade` header, one can set the `:cascade` option to `false`. + def cascade? + version_options[:cascade] + end + + def parameter_key + version_options[:parameter] + end + + def vendor + version_options[:vendor] + end + + def error_headers + cascade? ? { Grape::Http::Headers::X_CASCADE => 'pass' } : {} + end + + def potential_version_match?(potential_version) + versions.blank? || versions.any? { |v| v.to_s == potential_version } + end + + def version_not_found! + throw :error, status: 404, message: '404 API Version Not Found', headers: { Grape::Http::Headers::X_CASCADE => 'pass' } + end + end + end + end +end diff --git a/lib/grape/middleware/versioner/header.rb b/lib/grape/middleware/versioner/header.rb index 11cbfc7bcb..a34a80fc7f 100644 --- a/lib/grape/middleware/versioner/header.rb +++ b/lib/grape/middleware/versioner/header.rb @@ -22,8 +22,6 @@ module Versioner # X-Cascade header to alert Grape::Router to attempt the next matched # route. class Header < Base - include VersionerHelpers - def before match_best_quality_media_type! do |media_type| env.update( diff --git a/lib/grape/middleware/versioner/param.rb b/lib/grape/middleware/versioner/param.rb index 0c8f88a47c..771faf616b 100644 --- a/lib/grape/middleware/versioner/param.rb +++ b/lib/grape/middleware/versioner/param.rb @@ -19,8 +19,6 @@ module Versioner # # env['api.version'] => 'v1' class Param < Base - include VersionerHelpers - def before potential_version = Rack::Utils.parse_nested_query(env[Rack::QUERY_STRING])[parameter_key] return if potential_version.blank? diff --git a/lib/grape/middleware/versioner/path.rb b/lib/grape/middleware/versioner/path.rb index c824f2df81..dd4379767e 100644 --- a/lib/grape/middleware/versioner/path.rb +++ b/lib/grape/middleware/versioner/path.rb @@ -17,8 +17,6 @@ module Versioner # env['api.version'] => 'v1' # class Path < Base - include VersionerHelpers - def before path_info = Grape::Router.normalize_path(env[Rack::PATH_INFO]) return if path_info == '/' diff --git a/lib/grape/middleware/versioner_helpers.rb b/lib/grape/middleware/versioner_helpers.rb deleted file mode 100644 index 0cce2055cb..0000000000 --- a/lib/grape/middleware/versioner_helpers.rb +++ /dev/null @@ -1,75 +0,0 @@ -# frozen_string_literal: true - -module Grape - module Middleware - module VersionerHelpers - DEFAULT_PATTERN = /.*/i.freeze - DEFAULT_PARAMETER = 'apiver' - - def default_options - { - versions: nil, - prefix: nil, - mount_path: nil, - pattern: DEFAULT_PATTERN, - version_options: { - strict: false, - cascade: true, - parameter: DEFAULT_PARAMETER - } - } - end - - def versions - options[:versions] - end - - def prefix - options[:prefix] - end - - def mount_path - options[:mount_path] - end - - def pattern - options[:pattern] - end - - def version_options - options[:version_options] - end - - def strict? - version_options[:strict] - end - - # By default those errors contain an `X-Cascade` header set to `pass`, which allows nesting and stacking - # of routes (see Grape::Router) for more information). To prevent - # this behavior, and not add the `X-Cascade` header, one can set the `:cascade` option to `false`. - def cascade? - version_options[:cascade] - end - - def parameter_key - version_options[:parameter] - end - - def vendor - version_options[:vendor] - end - - def error_headers - cascade? ? { Grape::Http::Headers::X_CASCADE => 'pass' } : {} - end - - def potential_version_match?(potential_version) - versions.blank? || versions.any? { |v| v.to_s == potential_version } - end - - def version_not_found! - throw :error, status: 404, message: '404 API Version Not Found', headers: { Grape::Http::Headers::X_CASCADE => 'pass' } - end - end - end -end diff --git a/lib/grape/parser.rb b/lib/grape/parser.rb index a446b4da20..9dcb81ef3d 100644 --- a/lib/grape/parser.rb +++ b/lib/grape/parser.rb @@ -2,16 +2,14 @@ module Grape module Parser - module_function + extend Grape::Util::Registry - DEFAULTS = { - json: Grape::Parser::Json, - jsonapi: Grape::Parser::Json, - xml: Grape::Parser::Xml - }.freeze + module_function def parser_for(format, parsers = nil) - parsers&.key?(format) ? parsers[format] : DEFAULTS[format] + return parsers[format] if parsers&.key?(format) + + registry[format] end end end diff --git a/lib/grape/parser/base.rb b/lib/grape/parser/base.rb new file mode 100644 index 0000000000..56640d2e58 --- /dev/null +++ b/lib/grape/parser/base.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Grape + module Parser + class Base + def self.call(_object, _env) + raise NotImplementedError + end + + def self.inherited(klass) + super + Parser.register(klass) + end + end + end +end diff --git a/lib/grape/parser/json.rb b/lib/grape/parser/json.rb index 4e665a1ec2..a808999cce 100644 --- a/lib/grape/parser/json.rb +++ b/lib/grape/parser/json.rb @@ -2,14 +2,12 @@ module Grape module Parser - module Json - class << self - def call(object, _env) - ::Grape::Json.load(object) - rescue ::Grape::Json::ParseError - # handle JSON parsing errors via the rescue handlers or provide error message - raise Grape::Exceptions::InvalidMessageBody.new('application/json') - end + class Json < Base + def self.call(object, _env) + ::Grape::Json.load(object) + rescue ::Grape::Json::ParseError + # handle JSON parsing errors via the rescue handlers or provide error message + raise Grape::Exceptions::InvalidMessageBody.new('application/json') end end end diff --git a/lib/grape/parser/jsonapi.rb b/lib/grape/parser/jsonapi.rb new file mode 100644 index 0000000000..58e16571ba --- /dev/null +++ b/lib/grape/parser/jsonapi.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module Grape + module Parser + class Jsonapi < Json; end + end +end diff --git a/lib/grape/parser/xml.rb b/lib/grape/parser/xml.rb index 930c57f13e..bdb2a485fc 100644 --- a/lib/grape/parser/xml.rb +++ b/lib/grape/parser/xml.rb @@ -2,14 +2,12 @@ module Grape module Parser - module Xml - class << self - def call(object, _env) - ::Grape::Xml.parse(object) - rescue ::Grape::Xml::ParseError - # handle XML parsing errors via the rescue handlers or provide error message - raise Grape::Exceptions::InvalidMessageBody.new('application/xml') - end + class Xml < Base + def self.call(object, _env) + ::Grape::Xml.parse(object) + rescue ::Grape::Xml::ParseError + # handle XML parsing errors via the rescue handlers or provide error message + raise Grape::Exceptions::InvalidMessageBody.new('application/xml') end end end diff --git a/lib/grape/util/registry.rb b/lib/grape/util/registry.rb new file mode 100644 index 0000000000..6980445e68 --- /dev/null +++ b/lib/grape/util/registry.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Grape + module Util + module Registry + def register(klass) + short_name = build_short_name(klass) + return if short_name.nil? + + warn "#{short_name} is already registered with class #{klass}" if registry.key?(short_name) + registry[short_name] = klass + end + + private + + def build_short_name(klass) + return if klass.name.blank? + + klass.name.demodulize.underscore + end + + def registry + @registry ||= {}.with_indifferent_access + end + end + end +end diff --git a/lib/grape/validations.rb b/lib/grape/validations.rb index 9ae22ae6a1..fd33071d0f 100644 --- a/lib/grape/validations.rb +++ b/lib/grape/validations.rb @@ -2,29 +2,20 @@ module Grape module Validations + extend Grape::Util::Registry + module_function - def validators - @validators ||= {} - end + def require_validator(short_name) + raise Grape::Exceptions::UnknownValidator, short_name unless registry.key?(short_name) - # Register a new validator, so it can be used to validate parameters. - # @param short_name [String] all lower-case, no spaces - # @param klass [Class] the validator class. Should inherit from - # Grape::Validations::Validators::Base. - def register_validator(short_name, klass) - validators[short_name] = klass + registry[short_name] end - def deregister_validator(short_name) - validators.delete(short_name) - end + def build_short_name(klass) + return if klass.name.blank? - def require_validator(short_name) - str_name = short_name.to_s - validators.fetch(str_name) { Grape::Validations::Validators.const_get(:"#{str_name.camelize}Validator") } - rescue NameError - raise Grape::Exceptions::UnknownValidator, short_name + klass.name.demodulize.underscore.delete_suffix('_validator') end end end diff --git a/lib/grape/validations/contract_scope.rb b/lib/grape/validations/contract_scope.rb index 3b66df5722..218f47eec5 100644 --- a/lib/grape/validations/contract_scope.rb +++ b/lib/grape/validations/contract_scope.rb @@ -23,46 +23,12 @@ def initialize(api, contract = nil, &block) api.namespace_stackable(:contract_key_map, key_map) validator_options = { - validator_class: Validator, + validator_class: Grape::Validations.require_validator(:contract_scope), opts: { schema: contract, fail_fast: false } } api.namespace_stackable(:validations, validator_options) end - - class Validator < Grape::Validations::Validators::Base - attr_reader :schema - - def initialize(_attrs, _options, _required, _scope, opts) - super - @schema = opts.fetch(:schema) - end - - # Validates a given request. - # @param request [Grape::Request] the request currently being handled - # @raise [Grape::Exceptions::ValidationArrayErrors] if validation failed - # @return [void] - def validate(request) - res = schema.call(request.params) - - if res.success? - request.params.deep_merge!(res.to_h) - return - end - - raise Grape::Exceptions::ValidationArrayErrors.new(build_errors_from_messages(res.errors.messages)) - end - - private - - def build_errors_from_messages(messages) - messages.map do |message| - full_name = message.path.first.to_s - full_name << "[#{message.path[1..].join('][')}]" if message.path.size > 1 - Grape::Exceptions::Validation.new(params: [full_name], message: message.text) - end - end - end end end end diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb index 36c6ed4d3c..cb9b3f43e3 100644 --- a/lib/grape/validations/params_scope.rb +++ b/lib/grape/validations/params_scope.rb @@ -525,7 +525,7 @@ def derive_validator_options(validations) def validates_presence(validations, attrs, doc, opts) return unless validations.key?(:presence) && validations[:presence] - validate(:presence, validations.delete(:presence), attrs, doc, opts) + validate('presence', validations.delete(:presence), attrs, doc, opts) validations.delete(:message) if validations.key?(:message) end end diff --git a/lib/grape/validations/types/build_coercer.rb b/lib/grape/validations/types/build_coercer.rb deleted file mode 100644 index 5f15a1a0cb..0000000000 --- a/lib/grape/validations/types/build_coercer.rb +++ /dev/null @@ -1,92 +0,0 @@ -# frozen_string_literal: true - -module Grape - module Validations - module Types - module BuildCoercer - # Chooses the best coercer for the given type. For example, if the type - # is Integer, it will return a coercer which will be able to coerce a value - # to the integer. - # - # There are a few very special coercers which might be returned. - # - # +Grape::Types::MultipleTypeCoercer+ is a coercer which is returned when - # the given type implies values in an array with different types. - # For example, +[Integer, String]+ allows integer and string values in - # an array. - # - # +Grape::Types::CustomTypeCoercer+ is a coercer which is returned when - # a method is specified by a user with +coerce_with+ option or the user - # specifies a custom type which implements requirments of - # +Grape::Types::CustomTypeCoercer+. - # - # +Grape::Types::CustomTypeCollectionCoercer+ is a very similar to the - # previous one, but it expects an array or set of values having a custom - # type implemented by the user. - # - # There is also a group of custom types implemented by Grape, check - # +Grape::Validations::Types::SPECIAL+ to get the full list. - # - # @param type [Class] the type to which input strings - # should be coerced - # @param method [Class,#call] the coercion method to use - # @return [Object] object to be used - # for coercion and type validation - def self.build_coercer(type, method: nil, strict: false) - cache_instance(type, method, strict) do - create_coercer_instance(type, method, strict) - end - end - - def self.create_coercer_instance(type, method, strict) - # Maps a custom type provided by Grape, it doesn't map types wrapped by collections!!! - type = Types.map_special(type) - - # Use a special coercer for multiply-typed parameters. - if Types.multiple?(type) - MultipleTypeCoercer.new(type, method) - - # Use a special coercer for custom types and coercion methods. - elsif method || Types.custom?(type) - CustomTypeCoercer.new(type, method) - - # Special coercer for collections of types that implement a parse method. - # CustomTypeCoercer (above) already handles such types when an explicit coercion - # method is supplied. - elsif Types.collection_of_custom?(type) - Types::CustomTypeCollectionCoercer.new( - Types.map_special(type.first), type.is_a?(Set) - ) - else - DryTypeCoercer.coercer_instance_for(type, strict) - end - end - - def self.cache_instance(type, method, strict, &_block) - key = cache_key(type, method, strict) - - return @__cache[key] if @__cache.key?(key) - - instance = yield - - @__cache_write_lock.synchronize do - @__cache[key] = instance - end - - instance - end - - def self.cache_key(type, method, strict) - [type, method, strict].each_with_object(+'_') do |val, memo| - next if val.nil? - - memo << '_' << val.to_s - end - end - - instance_variable_set(:@__cache, {}) - instance_variable_set(:@__cache_write_lock, Mutex.new) - end - end - end -end diff --git a/lib/grape/validations/types/dry_type_coercer.rb b/lib/grape/validations/types/dry_type_coercer.rb index 1067eaf3a8..f9672198ee 100644 --- a/lib/grape/validations/types/dry_type_coercer.rb +++ b/lib/grape/validations/types/dry_type_coercer.rb @@ -22,16 +22,20 @@ class << self # collection_coercer_for(Array) # #=> Grape::Validations::Types::ArrayCoercer def collection_coercer_for(type) - Grape::Validations::Types.const_get(:"#{type.name.camelize}Coercer") + case type + when Array + ArrayCoercer + when Set + SetCoercer + else + raise ArgumentError, "Unknown type: #{type}" + end end # Returns an instance of a coercer for a given type def coercer_instance_for(type, strict = false) - return PrimitiveCoercer.new(type, strict) if type.instance_of?(Class) - - # in case of a collection (Array[Integer]) the type is an instance of a collection, - # so we need to figure out the actual type - collection_coercer_for(type.class).new(type, strict) + klass = type.instance_of?(Class) ? PrimitiveCoercer : collection_coercer_for(type) + klass.new(type, strict) end end diff --git a/lib/grape/validations/validators/base.rb b/lib/grape/validations/validators/base.rb index ee2dc4a87e..890963d9b7 100644 --- a/lib/grape/validations/validators/base.rb +++ b/lib/grape/validations/validators/base.rb @@ -59,10 +59,7 @@ def validate!(params) def self.inherited(klass) super - return if klass.name.blank? - - short_validator_name = klass.name.demodulize.underscore.delete_suffix('_validator') - Validations.register_validator(short_validator_name, klass) + Validations.register(klass) end def message(default_key = nil) diff --git a/lib/grape/validations/validators/contract_scope_validator.rb b/lib/grape/validations/validators/contract_scope_validator.rb new file mode 100644 index 0000000000..b8a3365c1b --- /dev/null +++ b/lib/grape/validations/validators/contract_scope_validator.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module Grape + module Validations + module Validators + class ContractScopeValidator < Base + attr_reader :schema + + def initialize(_attrs, _options, _required, _scope, opts) + super + @schema = opts.fetch(:schema) + end + + # Validates a given request. + # @param request [Grape::Request] the request currently being handled + # @raise [Grape::Exceptions::ValidationArrayErrors] if validation failed + # @return [void] + def validate(request) + res = schema.call(request.params) + + if res.success? + request.params.deep_merge!(res.to_h) + return + end + + raise Grape::Exceptions::ValidationArrayErrors.new(build_errors_from_messages(res.errors.messages)) + end + + private + + def build_errors_from_messages(messages) + messages.map do |message| + full_name = message.path.first.to_s + full_name << "[#{message.path[1..].join('][')}]" if message.path.size > 1 + Grape::Exceptions::Validation.new(params: [full_name], message: message.text) + end + end + end + end + end +end diff --git a/spec/grape/api/custom_validations_spec.rb b/spec/grape/api/custom_validations_spec.rb index d16c307fe2..6ed10527ac 100644 --- a/spec/grape/api/custom_validations_spec.rb +++ b/spec/grape/api/custom_validations_spec.rb @@ -35,7 +35,14 @@ def validate_param!(attr_name, params) end let(:app) { subject } - before { stub_const('Grape::Validations::Validators::DefaultLengthValidator', default_length_validator) } + before do + stub_const('DefaultLengthValidator', default_length_validator) + described_class.register(DefaultLengthValidator) + end + + after do + described_class.deregister(:default_length) + end it 'under 140 characters' do get '/', text: 'abc' @@ -77,7 +84,14 @@ def validate(request) end let(:app) { subject } - before { stub_const('Grape::Validations::Validators::InBodyValidator', in_body_validator) } + before do + stub_const('InBodyValidator', in_body_validator) + described_class.register(InBodyValidator) + end + + after do + described_class.deregister(:in_body) + end it 'allows field in body' do get '/', text: 'abc' @@ -113,7 +127,14 @@ def validate_param!(attr_name, _params) end let(:app) { subject } - before { stub_const('Grape::Validations::Validators::WithMessageKeyValidator', message_key_validator) } + before do + stub_const('WithMessageKeyValidator', message_key_validator) + described_class.register(WithMessageKeyValidator) + end + + after do + described_class.deregister(:with_message_key) + end it 'fails with message' do get '/', text: 'foobar' @@ -159,7 +180,14 @@ def access_header let(:app) { subject } let(:x_access_token_header) { 'x-access-token' } - before { stub_const('Grape::Validations::Validators::AdminValidator', admin_validator) } + before do + stub_const('AdminValidator', admin_validator) + described_class.register(AdminValidator) + end + + after do + described_class.deregister(:admin) + end it 'fail when non-admin user sets an admin field' do get '/', admin_field: 'tester', non_admin_field: 'toaster' @@ -218,7 +246,14 @@ def validate_param!(_attr_name, _params) end end - before { stub_const('Grape::Validations::Validators::InstanceValidatorValidator', validator_type) } + before do + stub_const('InstanceValidatorValidator', validator_type) + described_class.register(InstanceValidatorValidator) + end + + after do + described_class.deregister(:instance_validator) + end it 'passes validation every time' do expect(validator_type).to receive(:new).twice.and_call_original diff --git a/spec/grape/parser_spec.rb b/spec/grape/parser_spec.rb index ecc5fdfa4d..a349d61e73 100644 --- a/spec/grape/parser_spec.rb +++ b/spec/grape/parser_spec.rb @@ -3,20 +3,6 @@ describe Grape::Parser do subject { described_class } - describe 'DEFAULTS' do - subject { described_class::DEFAULTS } - - let(:expected_defaults) do - { - json: Grape::Parser::Json, - jsonapi: Grape::Parser::Json, - xml: Grape::Parser::Xml - } - end - - it { is_expected.to eq(expected_defaults) } - end - describe '.parser_for' do let(:options) { {} } diff --git a/spec/grape/validations/validators/contract_scope_validator_spec.rb b/spec/grape/validations/validators/contract_scope_validator_spec.rb new file mode 100644 index 0000000000..b8d462c313 --- /dev/null +++ b/spec/grape/validations/validators/contract_scope_validator_spec.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +describe Grape::Validations::Validators::ContractScopeValidator do + describe '.inherits' do + subject { described_class } + + it { is_expected.to be < Grape::Validations::Validators::Base } + end +end diff --git a/spec/grape/validations_spec.rb b/spec/grape/validations_spec.rb index 19dd130f03..5f983d2aa0 100644 --- a/spec/grape/validations_spec.rb +++ b/spec/grape/validations_spec.rb @@ -494,7 +494,8 @@ def validate_param!(attr_name, params) end before do - stub_const('Grape::Validations::Validators::DateRangeValidator', date_range_validator) + stub_const('DateRangeValidator', date_range_validator) + described_class.register(DateRangeValidator) subject.params do optional :date_range, date_range: true, type: Hash do requires :from, type: Integer @@ -515,6 +516,10 @@ def validate_param!(attr_name, params) end end + after do + described_class.deregister(:date_range) + end + context 'which is optional' do it "doesn't throw an error if the validation passes" do get '/optional', date_range: { from: 1, to: 2 } @@ -1186,7 +1191,14 @@ def validate_param!(attr_name, params) end end - before { stub_const('Grape::Validations::Validators::CustomvalidatorValidator', custom_validator) } + before do + stub_const('CustomvalidatorValidator', custom_validator) + described_class.register(CustomvalidatorValidator) + end + + after do + described_class.deregister(:customvalidator) + end context 'when using optional with a custom validator' do before do @@ -1338,8 +1350,8 @@ def validate_param!(attr_name, params) end before do - stub_const('Grape::Validations::Validators::CustomvalidatorWithOptionsValidator', custom_validator_with_options) - + stub_const('CustomvalidatorWithOptionsValidator', custom_validator_with_options) + described_class.register(CustomvalidatorWithOptionsValidator) subject.params do optional :custom, customvalidator_with_options: { text: 'im custom with options', message: 'is not custom with options!' } end @@ -1348,6 +1360,10 @@ def validate_param!(attr_name, params) end end + after do + described_class.deregister(:customvalidator_with_options) + end + it 'validates param with custom validator with options' do get '/optional_custom', custom: 'im custom with options' expect(last_response.status).to eq(200) diff --git a/spec/integration/dry_validation/dry_validation_spec.rb b/spec/integration/dry_validation/dry_validation_spec.rb index 6333bdacdd..d7b2f8efab 100644 --- a/spec/integration/dry_validation/dry_validation_spec.rb +++ b/spec/integration/dry_validation/dry_validation_spec.rb @@ -236,12 +236,4 @@ end end end - - describe Grape::Validations::ContractScope::Validator do - describe '.inherits' do - subject { described_class } - - it { is_expected.to be < Grape::Validations::Validators::Base } - end - end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 1589a0881d..06cb282a00 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -13,6 +13,10 @@ end end +Grape::Util::Registry.include(Deregister) +# issue with ruby 2.7 with ^. We need to extend it again +Grape::Validations.extend(Grape::Util::Registry) if Gem::Version.new(RUBY_VERSION).release < Gem::Version.new('3.0') + # The default value for this setting is true in a standard Rails app, # so it should be set to true here as well to reflect that. I18n.enforce_available_locales = true diff --git a/spec/support/deregister.rb b/spec/support/deregister.rb new file mode 100644 index 0000000000..f5dc7e5b23 --- /dev/null +++ b/spec/support/deregister.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module Deregister + def deregister(key) + registry.delete(key) + end +end